Skip to content
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

Dont mark content as BuiltIn if its not from the basegame #319

Merged
merged 2 commits into from
Nov 12, 2024

Conversation

Jarno458
Copy link
Contributor

@Jarno458 Jarno458 commented Nov 7, 2024

Currently SML lets mods register their content, content registration automatically traverses dependencies from research tree > schematic > recipe > item, this way if a mod for example adds a new recipe for iron plates it will be registered as if the mod owns it

After that SML will do a pass with all schematics from the base game, and it will force update anything referenced by these base game schematics to also be considered as part of the base game

This causes an issue when you set a mod schematic as an unlock dependency for an base game schematic, for example if a mod adds an milestone schematic that gives earlier access to pipes, and that mod wants to make sure that when that milestone is obtained the player can also buy the clear pipes variant, the mod would need to CDO edit the clear pipes unlock dependency to point to its mod milestone schematic, This however causes the mod milestone schematic to be force marked as part of the base game, and that results in it not getting properly registered and become unusable by the player

This PR makes sure that only content actually from the base game can be marked as coming from the base game

Fix for #248

@Jarno458
Copy link
Contributor Author

Jarno458 commented Nov 7, 2024

There are other ways to approach this issue, the solution i took here for me seems to score the best on cleanliness and least impactful to the current code. but it might not score high on performance, although i didn't notice any performance issues with it myself

Ideally i think it would be the best to refactor the mod content registry code so that objects are only registered once with the right flags and then never force updated, in that scenario both the OwnedByModReference and BuildIn flags would be set based on the package its location at the time the object is first registered rather then force updated later, but that would be a big refactoring that comes with its own risk and challenges that i like to avoid for now

@Jarno458 Jarno458 changed the base branch from master to dev November 8, 2024 14:36
Copy link
Member

@mircearoata mircearoata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a much simpler way to achieve the same solution:

If the Registration is not null, it will have its OwnedByModReference filled in with the mod that owns it, or FApp::GetProjectName() (which is FactoryGame in our case, but don't hardcode the project name) if it's part of the base game content. This also takes into account that base game content doesn't necessarily have to be under /Game/FactoryGame/, only under /Game/, and with potential DLCs in the future, having /Game/FactoryGame/ hardcoded would add an extra point of failure if those use another path.

And if the Registration is null, then no mod has registered that content, so it must be from the base game. In fact, with the current code, a null Registration can not happen, since GetAllowedNewFlags is only called after a FindOrAddObject, but this case should still be handled, just in case.

The result is a single if, similar to the Implicit one,, without any additional object package checks.

There is one way for OwnedByModReference to incorrectly be FactoryGame, and that is for things like adding a recipe to a vanilla milestone or a modded item to a vanilla recipe with a CDO edit. That is because when registering vanilla content, UModContentRegistry::FindContentOwnerFast always returns FApp::GetProjectName() as an optimization, since only unregistered vanilla content should reach that function during vanilla content registration.

So this case is not something to handle here, as mods should be registering "unreferenced" (meaning not referenced by a schematic listed in the GameWorldModule) recipes and items with the mod content registry, otherwise they would still show up as registered by FactoryGame, rather than by the mod that adds them, even if they don't have the BuiltIn flag set or the OwnedByModReference set to FactoryGame, and RegistrarModReference is arguably the more important field of the two, since that's the one that would be displayed by mods like TFIT.

@mircearoata mircearoata merged commit 8378d49 into satisfactorymodding:dev Nov 12, 2024
1 check failed
@Jarno458 Jarno458 deleted the fix_248 branch November 12, 2024 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants