BattletechModders / ModTek

Mod system for HBS's PC game BattleTech.
GNU Lesser General Public License v2.1
121 stars 34 forks source link

Can't merge overwritten content #93

Closed CptMoore closed 5 years ago

CptMoore commented 5 years ago

So BattleTech Enhanced overwrites existing base game items, e.g. Gear_HeatSink_Generic_Standard is added with ShouldMergeJSON = false. Then when a mod tries to merge stuff via the implicit StreamingAssets path, it will fail with the following message:

        Could not find an existing VersionManifest entry for Gear_HeatSink_Generic_Standard. Is this supposed to be a new entry? Don't put new entries in StreamingAssets!

This seems to be a new issue, as a guy claimed it worked before switching to my master build of ModTek (as part of testing new ME features).

How should ModTek behave here?

mpstark commented 5 years ago

This shouldn't happen, we'll have to track down where it's dying.

mpstark commented 5 years ago

This should be fixed in with commit 95604db7ba38b7fa9355e94ea3651bc0c34492b6 with BTRL stuff.

CptMoore commented 5 years ago

https://github.com/BattletechModders/ModTek/commit/95604db7ba38b7fa9355e94ea3651bc0c34492b6#r31917556

I think that will mess up advanced json merges, advanced json merges contain a TargetFile property which is the one with the path to the actual target file. You know better how to solve that issue (set the entry.id correctly in the first place? revert back to logic before? I don't know)

CptMoore commented 5 years ago

ok tested it, seems to actually be working, maybe the code I wrote before wasn't properly caching stuff in the first place and what you did fixed it

mpstark commented 5 years ago

I think that will mess up advanced json merges, advanced json merges contain a TargetFile property which is the one with the path to the actual target file. You know better how to solve that issue (set the entry.id correctly in the first place? revert back to logic before? I don't know)

I think it will be "more correct" to use the target file for the ID, and there is no longer a performance hit for checking really (since we're now caching the JObjects). I've changed it in 98894c742dc9e6e17056b8fdee87f80819a916d8

mpstark commented 5 years ago

Marking this as closed, since we've fixed it.