Barotrauma icon indicating copy to clipboard operation
Barotrauma copied to clipboard

new ugcId logic in master 0.19 matches when both are none

Open shangjiaxuan opened this issue 3 years ago • 2 comments

Disclaimers

  • [X] I have searched the issue tracker to check if the issue has already been reported.
  • [X] My issue happened while using mods.

What happened?

Originally the content path logic matches %ModDir:xxx% first with workshop id (ignores if not present, workshopid==0), then package name then alt-names.

Now that steamworkshopid logic is changed to use the new generic ugcid, but Option<T> of none will equal Option<T> of none. This means all local mod packages will match each other (and also match vanilla).

The code affected (Barotrauma/Barotrauma/BarotraumaShared/SharedSource/ContentManagement/ContentPath.cs, lines56-65):

                foreach (Identifier otherModName in otherMods)
                {
                    Option<ContentPackageId> ugcId = ContentPackageId.Parse(otherModName.Value);
                    ContentPackage? otherMod =
                        allPackages.FirstOrDefault(p => ugcId == p.UgcId)
                        ?? allPackages.FirstOrDefault(p => p.Name == otherModName)
                        ?? allPackages.FirstOrDefault(p => p.NameMatches(otherModName))
                        ?? throw new MissingContentPackageException(ContentPackage, otherModName.Value);
                    cachedValue = cachedValue.Replace(string.Format(OtherModDirFmt, otherModName.Value), Path.GetDirectoryName(otherMod.Path));
                }

Suggested change:

                        allPackages.FirstOrDefault(p => ugcId.IsSome() && ugcId == p.UgcId)

Reproduction steps

Have one local mod that references its contents with "%ModDir%" or "%ModDir:xxx%", and the resource will not load.

Bug prevalence

Happens every time I play

Version

0.19.10.0

-

No response

Which operating system did you encounter this bug on?

Windows

Relevant error messages and crash reports

I think things are clear enough.

shangjiaxuan avatar Oct 05 '22 18:10 shangjiaxuan

Thank you for the report, and for helping us diagnose this! You're right, this seems to be an oversight that slipped past us. Fixed in https://github.com/Regalis11/Barotrauma-development/commit/472c373f129ee56287d7c7657f66845547dd86da

Regalis11 avatar Oct 05 '22 18:10 Regalis11

Thanks for taking the time to close this, but we'll keep this open until we've tested it internally and verified it's working

Regalis11 avatar Oct 06 '22 11:10 Regalis11

Tested, working correctly. Closing.

Rokvach avatar Oct 18 '22 08:10 Rokvach