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

Check for duplicate files among enabled mods #669

Open
GeckoEidechse opened this issue Feb 14, 2024 · 15 comments
Open

Check for duplicate files among enabled mods #669

GeckoEidechse opened this issue Feb 14, 2024 · 15 comments
Assignees

Comments

@GeckoEidechse
Copy link
Member

GeckoEidechse commented Feb 14, 2024

Related to R2NorthstarTools/FlightCore#793

Basically some crashes in Northstar like audio system related ones are caused by the same audio file being overwritten twice.

Similar if two or more enabled mods replace the same script there's most likely gonna a be an issue.

As such, check between all enabled mods if they replace the same files and show a warning message if they do

@Alystrasz suggested to this in launcher directly instead ^^"

@Alystrasz Alystrasz self-assigned this Feb 14, 2024
@Alystrasz
Copy link
Contributor

(progress tracked here: https://github.com/Alystrasz/NorthstarLauncher/tree/feat/audio-multioverride-detection)

At this point, I can detect two mods trying to override the same audio file, logging a warning message before Northstar crashes; should I also prevent file override by mod B if it was already overriden by mod A?

Also, that's the first time I hear of a script override issue, does anybody already encountered this problem?

@ASpoonPlaysGames
Copy link
Contributor

Also, that's the first time I hear of a script override issue, does anybody already encountered this problem?

Yup, some common files get overriden by mods a lot, with most of them exposing a new global function or something, causing compile errors on conflict.

I think an example would be halo announcer + client kill callback maybe?

@GeckoEidechse
Copy link
Member Author

should I also prevent file override by mod B if it was already overriden by mod A?

Works for me. I would just make sure that if there's two mods that override the same file we should still prevent the game from launching somehow.

That way the player has to fix it now instead of just forever having two conflicting mods spewing warning messages but they keep ignoring them ^^

@EM4Volts
Copy link
Contributor

quoting gecko Works for me. I would just make sure that if there's two mods that override the same file we should still prevent the game from launching somehow.

i think this is the wrong way of handling it.
Instead of not launching the game when two mods have the same file there should just be a proper rework on LoadPriority.

remove load priority from the mod.json so its not set by mod authors. instead set it on northstar launch in a loadorder file, that file can be editited by people themelves or mod managers easily and northstar just adds new mods that arent in there to the file on launch.

a single file for the entirety of a users mods.

if a mod overwrites something it should overwrite it. thats how patch mods for certain issues work, two mods overwriting the same script should never result in the game not launching.

for audio files this sort of makes sense todo but also not. the last mod in the loadorder should just be the one loaded.

@EM4Volts
Copy link
Contributor

to append to my reply:
this isnt an issue of northstar, instead its an issue by the user. northstar should not automatically not work just because two mods overwrite the same file

@ASpoonPlaysGames
Copy link
Contributor

At this point, I can detect two mods trying to override the same audio file, logging a warning message before Northstar crashes; should I also prevent file override by mod B if it was already overriden by mod A?

Honestly it would be better to just fix the crash rather than prevent mod loading or whatever, multiple mods should be able to add audio overrides to the same audio event (one event can have multiple source files, so we should be able to just combine them)

@ASpoonPlaysGames
Copy link
Contributor

(progress tracked here: https://github.com/Alystrasz/NorthstarLauncher/tree/feat/audio-multioverride-detection)

Also, it looks like you are using the json file name to determine collisions, are we sure that that's accurate? Is it two audio overrides with the same file structure that causes crashes, or two audio overrides that override the same audio event (which is specified within the json)

@GeckoEidechse
Copy link
Member Author

GeckoEidechse commented Feb 22, 2024

quoting gecko Works for me. I would just make sure that if there's two mods that override the same file we should still prevent the game from launching somehow.

i think this is the wrong way of handling it.

To maybe elaborate on my initial comment, my main concern is trying to resolve partially broken setups and in the process introducing more complexity.

To maybe better explain what I mean, let me mention my favourite example on this which is VTOL patching broken mods. In the past VTOL would attempt to fix incorrectly formatted mods during installation.
This would even work for the most part but gave us the result, that modders (who primarily used VTOL) would upload incorrectly formatted mods to Thunderstore and on testing with VTOL would not notice any issues.

Meanwhile, Viper (which was more popular with players/non-modders) would not have this functionality that would attempt to correct the incorrectly formatted mod and as such mods would fail to install on there (same on r2mm and FlightCore as well). Resulting in players not being able to install mods while modders were under the impression that their mod was formatted correctly. 1

Basically, what I was trying to say with my original comment is that, if anything is considered incorrect, fail fast, safely, and early to indicate an issue. If there's a crash cause we don't handle it properly under the hood, we should fix the underlying issue. ^^

Footnotes

  1. And no, requiring all mod-managers to add some correcting functionality was not a solution here as the it is entirely heuristic based and a major factor for some of the bugs in VTOL. The real solution ofc is checks during mod upload but that's a different story I really don't wanna go into rn...

@GeckoEidechse
Copy link
Member Author

GeckoEidechse commented Feb 22, 2024

Instead of not launching the game when two mods have the same file there should just be a proper rework on LoadPriority.

Can you add this to R2Northstar/Northstar#618 ?

@EM4Volts
Copy link
Contributor

quoting gecko Works for me. I would just make sure that if there's two mods that override the same file we should still prevent the game from launching somehow.
i think this is the wrong way of handling it.

To maybe elaborate on my initial comment, my main concern is trying to resolve partially broken setups and in the process introducing more complexity.

To maybe better explain what I mean, let me mention my favourite example on this which is VTOL patching broken mods. In the past VTOL would attempt to fix incorrectly formatted mods during installation. This would even work for the most part but gave us the result, that modders (who primarily used VTOL) would upload incorrectly formatted mods to Thunderstore and on testing with VTOL would not notice any issues.

Meanwhile, Viper (which was more popular with players/non-modders) would not have this functionality that would attempt to correct the incorrectly formatted mod and as such mods would fail to install on there (same on r2mm and FlightCore as well). Resulting in players not being able to install mods while modders were under the impression that their mod was formatted correctly. 1

Basically, what I was trying to say with my original comment is that, if anything is considered incorrect, fail fast, safely, and early to indicate an issue. If there's a crash cause we don't handle it properly under the hood, we should fix the underlying issue. ^^

Footnotes

  1. And no, requiring all mod-managers to add some correcting functionality was not a solution here as the it is entirely heuristic based and a major factor for some of the bugs in VTOL. The real solution ofc is checks during mod upload but that's a different story I really don't wanna go into rn...

that issue u mentioned is based on not properly formatted mods, not on multiple mods containing the same script/files tho?
dont see how that is relevant.
northstar should not just stop working simply because two mods add the same file,
theres mods that are literally designed to overwrite the script of another mod this way which just load AFTER the original mod thus work.
this isnt a norhtstar issue but once again a user issue that can not be automatically solved properly

@EM4Volts
Copy link
Contributor

Instead of not launching the game when two mods have the same file there should just be a proper rework on LoadPriority.

Can you add this to R2Northstar/Northstar#618 ?

no, i no longer engage with that topic. my last words on that have been said.

@ASpoonPlaysGames
Copy link
Contributor

no, i no longer engage with that topic. my last words on that have been said.

not really a discussion then is it?

@GeckoEidechse
Copy link
Member Author

GeckoEidechse commented Feb 22, 2024

that issue u mentioned is based on not properly formatted mods, not on multiple mods containing the same script/files tho?
dont see how that is relevant.

I was simply giving an example why failing early is important in the right situation.

 

northstar should not just stop working simply because two mods add the same file,
theres mods that are literally designed to overwrite the script of another mod this way which just load AFTER the original mod thus work.

I'm 100% in agreement with you and @ASpoonPlaysGames on this, which is why I wrote:

If there's a crash cause we don't handle it properly under the hood, we should fix the underlying issue. ^^

@Alystrasz
Copy link
Contributor

Also, it looks like you are using the json file name to determine collisions, are we sure that that's accurate? Is it two audio overrides with the same file structure that causes crashes, or two audio overrides that override the same audio event (which is specified within the json)

We can discuss that again on the PR when it's opened (soon)!
I think I'll address audio overrides and script overrides in separate PRs.

@BigSpice
Copy link

BigSpice commented Feb 22, 2024

To maybe better explain what I mean, let me mention my favourite example on this which is VTOL patching broken mods. In the past VTOL would attempt to fix incorrectly formatted mods during installation. This would even work for the most part but gave us the result, that modders (who primarily used VTOL) would upload incorrectly formatted mods to Thunderstore and on testing with VTOL would not notice any issues.

I added warning to checks to modders back in 1.20.0.
Issues like above should not be prevalent anymore, you recieve a big warning banner on trying to export.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

5 participants