GlowPuff / ImperialCommander2

A companion app for the Imperial Assault board game.
MIT License
88 stars 12 forks source link

Not all banned groups loaded in some missions #49

Closed Noldorion closed 1 year ago

Noldorion commented 1 year ago

There's a weird bug with some missions where IC2 does not load all banned groups that are given in the mission JSON.

Example: The mission "The Source" has 16 banned groups in the JSON, but only 12 are shown when the mission is selected in IC2.

When the mission is loaded into the newest ICE version and saved, all banned groups appear as they should, so it's probably a file format issue.

GlowPuff commented 1 year ago

When an "official" Mission is selected in Setup, the ignored groups are collected from the Mission preset JSON files, NOT from the Mission file itself. If you look at the Mission preset (\Assets\Resources\MissionPresets\core.json) for the Core10 Mission you mentioned, the preset only lists 12 groups to ignore. And this is what IC2 shows. The actual Mission file for core10, on the other hand, has 16 groups in the "bannedGroups" array. The Mission preset data for that Mission, and possibly others, is incorrect.

Custom Missions, on the other hand, DO gather their ignored groups from the Mission file itself, since they do not have a Mission preset to refer to. When you re-save the core10 Mission in ICE and load it into IC2 as a custom Mission, it's "correctly" getting the full 16 groups to ignore right from the Mission file.

So this is a good thing because it means it's not a data format issue. Rather, it's the data itself that is incorrect, which is much easier to fix. So the question is, should all the Mission presets be re-evaluated? Or should I just have IC2 get the ignored groups from the Mission file itself during Setup? I'm leaning towards getting the data from the Mission file itself, and here's why:

Presets are a leftover from the original IC1 Classic mode, which has no Mission files to get ignored groups and other data from. Presets contain initial groups, reserved groups, factions, and other data needed to run a Mission in IC1. For Classic, the Mission preset is the "source of truth" for running a Mission. As you know, this same data is also a part of every Mission file created in ICE. Since presets are a relic from IC1, it makes sense to wean off this data as the "source of truth" and rely only on the Mission files themselves. I mean, that's why ICE has properties for all that data in the first place. The old presets are not needed for IC2 at all (*), except for Classic mode.

(*) - Not 100% true, I actually lookup some Mission data from presets in the Campaign screen, such as the Threat level. I do this because it's quicker. Otherwise I'd have to load, parse, and validate the entire Mission just to get the threat level and show it on the screen.

Noldorion commented 1 year ago

Haha, God, I feel so stupid. I remember that we actually talked about this some time ago, and I just forgot it. Yes, there are differences between the two lists. In the Mission Presets, I was a bit more cautious about banning groups, because IC1 was much more built for being flexible. I reworked all those things for IC2.

I wholeheartedly agree with you that the mission presets are a leftover, and that it would be better to pull all the info from the mission file itself. The thing is, I think some of the information is not present in the mission JSON at all. You already mentioned the default threat level, and that is something that's not in the mission file anywhere, I think. So we still need that info somewhere.

I think at some point in the future, when we remove Classic altogether, it might make sense to scrub the mission presets off anything no longer needed, and possibly integrate anything that is still relevant into a different file. For now, pulling the banned groups from the mission JSON will be best.

Noldorion commented 1 year ago

Addendum: Of course, we have all the necessary threat levels in the campaign data. So I think we could remove the mission presets altogether: Threat level is set by the campaign manager, reserved / initial / banned groups my the mission JSON. No need to change or add anything. The only thing is that there would be no default threat level when starting a one-off Saga mission. But in that case, I think it's fine if the app just defaulted to a threat level of 2, and players could set their own threat level.

What do you think?

GlowPuff commented 1 year ago

Yes, that's right, the threat is already in the campaign data! I think this is a good idea! For the next update I'll remove all dependencies on the old preset data.