-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Creation parsing and fix issues #25
Conversation
- Read creations from LocalAppData - Don't include CC files in primary plugins - Don't parse CCC file if sTestFile set - Fix parsing of secondary data directories
- Fix issues with core plugin LO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not own Starfield so I am not sure what the purpose of this is but I think 1) the parsing section of code should be moved to dedicated functions and 2) this should be more commented / documented than it is right now.
There is no default CCC file in Starfield. ContentCatalog.txt is the only way to look up what mods constitute Creation mods. However the CCC file will still be parsed if it exists. Crucially, core plugins are not auto-loaded before plugins.txt. One workaround is to put the core plugins into a new Starfield.ccc file which forces the game to load them first. So this function must parse both in order to determine what we're considering Creations. However we don't want it to duplicate the core plugins if they get put in there as this causes MO2 to bug out trying to load duplicate core plugins. I guess I can add some comments but I don't really see the point of moving the parsing code out. This is the one place it needs to be, and pulling it from the unmanaged mods is a bit odd. I don't think it makes sense to make it part of the interfaces. Perhaps it could be recast as the Starfield class. |
Actually I just realized there's an issue in the CCC parsing code. It should be adding the files if they aren't core plugins. I'm surprised that didn't trigger an issue, honestly. I'll fix that when I'm back at my computer. |
src/starfieldunmanagedmods.cpp
Outdated
} | ||
|
||
QString StarfieldUnmangedMods::displayName(const QString& modName) const | ||
{ | ||
QFile content(m_AppDataFolder + "/" + game()->gameShortName() + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this declaration can be removed.
src/starfieldunmanagedmods.cpp
Outdated
"/ContentCatalog.txt"); | ||
QString name = modName; | ||
QJsonObject contentObj = getContentCatalog(); | ||
for (auto mod : contentObj.keys()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be const auto&
(applies to most for
-loop I think).
src/starfieldunmanagedmods.h
Outdated
class StarfieldUnmangedMods : public GamebryoUnmangedMods | ||
{ | ||
public: | ||
StarfieldUnmangedMods(const GameGamebryo* game); | ||
StarfieldUnmangedMods(const GameStarfield* game, const QString appDataFolder); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing &
for appDataFolder
.
src/starfieldunmanagedmods.cpp
Outdated
QStringList StarfieldUnmangedMods::secondaryFiles(const QString& modName) const | ||
{ | ||
QStringList files; | ||
QJsonObject contentObj = getContentCatalog(); | ||
for (auto mod : contentObj.keys()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as below: can iterate items directly and should use const auto&
.
src/starfieldunmanagedmods.cpp
Outdated
"/ContentCatalog.txt"); | ||
if (content.exists()) { | ||
if (content.open(QIODevice::OpenModeFlag::ReadOnly)) { | ||
ON_BLOCK_EXIT([&content]() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be removed.
src/starfieldunmanagedmods.cpp
Outdated
{ | ||
QFile content(m_AppDataFolder + "/" + game()->gameShortName() + | ||
"/ContentCatalog.txt"); | ||
if (content.exists()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would reverse the if
to avoid the three nested levels:
if (!content.exists()) {
return {};
}
if (!content.open(QIODevice::OpenModeFlag::ReadOnly)) {
return {};
}
// ...
src/gamestarfield.cpp
Outdated
for (auto mod : contentObj.keys()) { | ||
if (mod == "ContentCatalog") | ||
continue; | ||
auto modData = contentObj.value(mod).toObject(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After reading everything, I think this should be refactored somewhere so that getContentCatalog
returns a custom structure with appropriate type rather that the QJsonObject
. That would make using it in other functions more straightforward.
I'll make some of these changes when I can. I doubt I even need the exists checks as open will simply return false. I suppose I can feed the data into a basic structure with just the files and mod name which would skip having to ignore the 'ContentCatalog' object and simplify loading the various mod data types. The main thing is restructuring it to use the plugin filename as the key. |
- Fix typo in unmanaged mods class name - Add struct to store content catalog data - Fetch relevant info in content catalog parser - Use unmanaged mods parseer in main game class (for CC plugins) - for loop fixes
TODO: Consolidate ContentCatalog.txt parsing somewhat?