0023271: Better error message when loading mod'd save game without mod installed - DEV Arcen

ID	Project	Category		Date Submitted	Last Update

0023271	AI War 2	Suggestion		Jun 9, 2020 5:55 pm	Jun 20, 2020 4:35 pm

Reporter	BadgerBadger	Assigned To	Chris_McElligottPark
Status	resolved	Resolution	fixed
Product Version	Beta 2.063 Fixes and Tweaks
Fixed in Version	Beta 2.078 Mod and Expansion Prepping

Summary	0023271: Better error message when loading mod'd save game without mod installed
Description	At one point I tried to open a save from a modded game without the mod installed, and I hit the following error 6/8/2020 8:51:12 PM Faction deserialization error at stage 17500 from serialized version 2.062 loading into new version 2.062, error: System.ArgumentNullException: Value cannot be null. Parameter name: key at System.Collections.Generic.Dictionary`2[TKey,TValue].FindEntry (TKey key) [0x00008] in <1f0c1ef1ad524c38bbc5536809c46b48>:0 at System.Collections.Generic.Dictionary`2[TKey,TValue].ContainsKey (TKey key) [0x00000] in <1f0c1ef1ad524c38bbc5536809c46b48>:0 at Arcen.Universal.ArcenSparseLookup`2[K,T].SetItemForKey (K Key, T Item) [0x00001] in <dab07efe3ee548a1b4d2b5e2988568c6>:0 at Arcen.Universal.ArcenSparseLookup`2[K,T].set_Item (K key, T value) [0x00001] in <dab07efe3ee548a1b4d2b5e2988568c6>:0 at Arcen.AIW2.External.ShipStats.DeserializeFrom (Arcen.Universal.ArcenDeserializationBuffer Buffer) [0x00199] in <039fa862d75e408dab3a4044a41d18b3>:0 at Arcen.AIW2.External.ExternalData_MinorFactionCommon.DeserializeExternalData (System.Object ParentObject, System.Object[] Target, System.Int32 ItemsToExpect, Arcen.Universal.ArcenDeserializationBuffer Buffer) [0x000a5] in <039fa862d75e408dab3a4044a41d18b3>:0 at Arcen.Universal.ArcenExternalData.DeserializeFrom (System.Object ParentObject, Arcen.Universal.ArcenDeserializationBuffer Buffer) [0x00184] in <dab07efe3ee548a1b4d2b5e2988568c6>:0 at Arcen.Universal.ArcenExternalDataLookup.DeserializeFrom (System.Object ParentObject, Arcen.Universal.ArcenDeserializationBuffer Buffer) [0x0001d] in <dab07efe3ee548a1b4d2b5e2988568c6>:0 at Arcen.AIW2.Core.Faction.DeserializeFrom (Arcen.Universal.ArcenDeserializationBuffer Buffer) [0x008b7] in <7dd00644530c4b05a4b0e384a0c6e41b>:0 (and then a number of other errors). We need a good error message when trying to load a modded save game to make it easier for devs to figure out what's going on. And so I don't panic and worry we broke save games or something.
Tags	No tags attached.

Chris_McElligottPark Jun 19, 2020 5:49 pm administrator ~0057396	This was supposed to do it, but does not quite yet: * The "ForExpansion" field on the dynamic table rows has been removed. It was not being properly set in a lot of cases, anyhow. Now dynamic table rows have DataSource RowFrom, Expansion RowFromExpansion, and XmlMod RowFromXmlMod. These let you see if the row was originally created from an expansion, the base game, programmatically, or from an xml mod. If an expansion or an xml mod, then it links to the mod. Note that variants of these things (copy_from, etc) would wind up with a value that is whatever the variant was created in. If a mod makes a variant, then the original would still be from an expansion or the base game, but the new variant would show up as being from the specific mod. Basically what you would expect. Note that partial records do NOT overwrite the value. So if there's something from the base game, and then a mod uses a partial record to change data, it would still show up as being a base game item. * THAT said, there is a new List<RefThreeTuple<DataSource, Expansion, XmlMod>> RowAlteredByPartialRecords on dynamic table rows, which lets you track exactly how many partial records have contributed to it, and if they were from the base game, expansions, or mods. Note that these row alterations are not copied over to copy_from entries. So if one mod alters an entry a bunch via partial records, and then another mod makes a variant, the variant would not have a record of the partial records that went into what it was copied from. If you needed to find that for whatever reason, you could check the existing CopiedFrom attribute on the variant row. That has a chain all the way back to the original row, if you need it. * GetRowByName() on dynamic tables used to require three parameters -- name (stil there), create-if-missing (gone now), and expansion (gone now and was never used). Mods will have to be updated to account for the new method style and behavior, and hopefully the core game itself is also handling this properly now (but there may be bugs in the short term). This was a VERY old way of handling this, before we even had proper formal xml mod support, and there were some implicit behaviors that were problematic. There were various overloads of this method that would complain if it could not find a row by name, but the default was for it to just silently return null. This could then lead to unexpected invisible problems. * This now throws a MissingArcenDynamicTableRowException instead of returning null if the name can't be found. * The game now uses the MissingArcenDynamicTableRowException in several key points during deserialization (loading a savegame, in other words) to let us show a message stating "hey, this savegame is talking about something called X, but that doesn't exist, so you may not have the same mods and/or expansions that the savegame was using). ** Previously if it couldn't find things, it would either just spam errors silently and then show one final parsing error that made no sense whatsoever to anyone, or it would try to patch over things by loading a "dummy ship" in without the actual stats on it. The latter is completely gone, because that was going to absolutely break multiplayer but in very inconvenient-to-figure-out ways. * For the sake of flexibility and retaining prior functionality, there are a couple of new methods on arcen dynamic tables beyond just GetRowByName(). CreateNewRow() has been added for if you want to create a row programmatically from whatever source. * Previously the only way to explicitly programmatically create a row was to call GetRowByName() with an invalid name and create-if-missing true. That felt very hacky, and it made code that used it (maybe two lines in the whole codebase) very hard to read. * Additionally, this was kind of the only real reason for ever passing in the Expansion field, since it would then set the expansion on the created field. That didn't justify us passing in lots of mysterious nulls all around the codebase. * This method now, naturally, takes in string Name, DataSource Source, Expansion ForExpansion, XmlMod ForXmlMod, so that you can fully document the row's origins as you wish, just like any other row. * This is still a kind of method that we almost never should need to use, because the whole purpose is to let these dynamic tables be initialized and filled by xml itself. GetRowByNameOrNullIfNotFound() works like GetRowByName() used to, in that it will return null if it can't find the row, rather than throwing an exception. * There are actually a LOT of places in the codebase, particularly in factions and commands, where we use this. Probably a lot of mods will want to use this, also. * The purpose of this is basically when you want to "find it if it's there, or else handle the fact that it isn't there in an intelligent way." When you have that sort of scenario, using exception handling to control program flow would be both crude and slow and hard to read. * There are a number of places where this method is now used in the external part of the codebase, and if the result is null it generates some other sort of error or just does something else and isn't even an error case. * There are some other places in the external part of the codebase where we still use GetRowByName() instead, but we took out the follow-up exceptions where it used to say "if the result is null, then throw an exception." The new exceptions automatically happen, saving code, and are more detailed. ** GetRowByNameOrCreateIfNotFound() is really not used frequently, but is nice to have just in case. We use it a couple of places. It does what you would expect. * Overall we may have some short-term bugs from these changes, but they should also help us -- and modders -- avoid "invisible errors" that are way harder to track down. ** These changes also make the code more readable, and give equal precedence to mods and expansions in terms of figuring out "what came from where." * Speaking of invisible errors, we found some places where dynamic tables were trying to reference other dynamic table rows in the wrong order of operations for initialization, and that was potentially causing invisible errors before (or certainly wrong data). These now errored during game startup, so we were able to fix them: MapTypeDataTable was being referenced by ScenarioDataTable, so had to move to come before it. GameEntityTypeDataTable was being referenced by TestChamberTable, so TestChamberTable had to move below that. SpecialFactionDataTable was referencing TeamColorDefinitionTable, so that had to move up (and we took TeamColorPrefabsTable with it). These somewhat fall under the category of "how did these ever work?" * We then had some other problems relating to entities and the order in which they were present in xml, and they were working fine but it was kind of luck that they did. Something like the WarspiteArtillery, for instance, would be defined prior to the Warspite, which is something it would spawn on death. The reason that this has historically worked is that it would create a stub entry for Warspite as soon as it was entered, and then later that would get filled in by the actual entity. But... what if the actual entity was never filled in? We'd have zero way of knowing, and the game would just proceed with this practically-null ship. So it was time to change how these are read in, so that we actually get proper errors for the latter case, but can still order our xml how we want. We now load three fields where entity type datas referenced other entity type datas in a delayed fashion, so that none of this happens. If there's an error in xml, it will no longer be invisible. * Added a new debug setting to personal settings: Write Savegame Serialization Logs In Realtime This is used only on conjunction with the main 'Write Savegame Serialization Logs', and should almost never be used. This causes the log process to write to disk immediately as it does every log action, rather that writing to a buffer and then writing the results of that buffer at the end. This is one of those features that you should use if you're needing to test a deserialization that is crashing the program, which should basically never happen if you're not a core developer of the game. As you may surmise, our first result after getting the game functional again after all the above changes was that any savegame was crashing the program with no error messages, so hence this. * The ArcenSerializationTester.Writer object has been made private, and now you just call Append or AppendLine on ArcenSerializationTester directly, so it can work in the prior style or the new realtime format.

Chris_McElligottPark Jun 19, 2020 6:18 pm administrator ~0057397	Better! * The exceptions you get now when you try to load a savegame that had a mod or expansion in it that you are missing now gives an error message that is brief and actually makes it pretty clear why this is happening. It doesn't say what mods or expansions you are missing yet, but that's another next step. ** This particular fix catches even "mods that are just changed files with added stuff" and not just properly set up mods that someone put in their own folder.

Chris_McElligottPark Jun 20, 2020 4:35 pm administrator ~0057411	Handling the error messages, plus more. More to come, although it's beyond the scope of this particular issue. But in a multiplayer landscape, we really do need a few things like the ability to turn on and off mods (if they CAN be). * Savegames have kept track of which expansions are enabled in them for a long time now, but until now they did not stop you from trying to load a savegame that used an expansion you did not have. ** Older quickstarts that had expansions on (usually just with beacons) but no expansion content enabled will still be usable by non-expansion players. * Savegames have never tracked which mods are enabled for them, but now they do. They also now prevent you from loading a savegame with a mod enabled unless you also have that mod enabled. This sort of thing, like the expansions, gets a lot more relevant now that multiplayer is coming up, but it's also useful for our own internal testing. * In the case of a savegame that you are trying to load that you are missing some expansion or mods with, it gives you a complete list of what you are missing, now -- assuming that the save was created in version 2.078 or later. * The in-game escape menu now shows what expansions or mods are enabled in the savegame you are playing, if any are. * Under the hood, GetExpansionStatus_SemiExpensive() has been removed, as it was slow and also kind of confusing. * Added ConvertToCondensedFormat() as an extension method on strings, and it makes a unicode-style string into a condensed-format-viable string. ** This is useful for mods that might have names that extend out of the ASCII name range, particularly for international modders. * Added SplitIntoWordsByCapitalization() as an extension on strings. We are now using this to generate automatic "display names" for xml mods. So for instance "BobsFancyMod" becomes "Bobs Fancy Mod" and "UTTTransport8" would become "UTT Transport8" * When you are loading the game, it now shows any expansions or mods you have installed at the top of the list of things it is loading. Additionally, on the main menu if you hover over the "load time" entry at the bottom right, it shows those again. We definitely predict that there would have been confusion among people with mods and expansions that don't match who are trying to play multiplayer.

Date Modified	Username	Field	Change
Jun 9, 2020 5:55 pm	BadgerBadger	New Issue
Jun 9, 2020 5:55 pm	BadgerBadger	Status	new => assigned
Jun 9, 2020 5:55 pm	BadgerBadger	Assigned To	=> Chris_McElligottPark
Jun 9, 2020 5:55 pm	BadgerBadger	Relationship added	related to 0023269
Jun 9, 2020 5:56 pm	BadgerBadger	Description Updated
Jun 9, 2020 6:06 pm	BadgerBadger	Summary	Better error message when loading mod'd save game => Better error message when loading mod'd save game without mod installed
Jun 9, 2020 6:06 pm	BadgerBadger	Description Updated
Jun 19, 2020 5:49 pm	Chris_McElligottPark	Note Added: 0057396
Jun 19, 2020 6:18 pm	Chris_McElligottPark	Note Added: 0057397
Jun 20, 2020 4:35 pm	Chris_McElligottPark	Status	assigned => resolved
Jun 20, 2020 4:35 pm	Chris_McElligottPark	Resolution	open => fixed
Jun 20, 2020 4:35 pm	Chris_McElligottPark	Fixed in Version	=> Beta 2.078 Mod and Expansion Prepping
Jun 20, 2020 4:35 pm	Chris_McElligottPark	Note Added: 0057411