Opened 4 months ago

Closed 4 months ago

#6553 closed defect (fixed)

Unknown string loaded from matchsettings.json disables the game

Reported by: Langbart Owned by: wraitii
Priority: Must Have Milestone: Alpha 26
Component: UI – Game setup Keywords: regression
Cc: Patch: Phab:D4678

Description (last modified by Langbart)

With [26913] the string for Conquest Civic Centers was changed which caused a problem for gameboy.

to reproduce

  • Open your existing matchsettings.json and modify a string under VictoryConditions e.g. "conquest" => "con"
  • Open the game (make sure the option Persist match settings is enabled)
  • Select Single Player => Matches
  • Errors should fill the screen

errors

0 A.D. (0.0.26) Main log (warnings and errors only)
ERROR: JavaScript error: gamesettings/attributes/VictoryConditions.js line 73 this.conditions[name] is undefined _add@gamesettings/attributes/VictoryConditions.js:73:7 fromInitAttributes@gamesettings/attributes/VictoryConditions.js:38:10 fromInitAttributes@gamesettings/GameSettings.js:90:17 parseSettings@gui/gamesetup/Controllers/GameSettingsController.js:191:18 onLoad@gui/gamesetup/Controllers/GameSettingsController.js:89:10 SetupWindow@gui/gamesetup/SetupWindow.js:61:11 init@gui/gamesetup/gamesetup.js:47:18 onPress@gui/pregame/MainMenuItems.js:99:13 performButtonAction@gui/pregame/MainMenuItemHandler.js:77:9 pressButton@gui/pregame/MainMenuItemHandler.js:63:10

ERROR: GUI page 'page_gamesetup.xml': Failed to call init() function

possible solution

Check if the value loaded from the matchsettings.json is real.
Example: gamesettings/attributes/VictoryConditions

    fromInitAttributes(attribs)
    {
        let legacy = this.getLegacySetting(attribs, "VictoryConditions");
+       const checkForExistence = legacy.filter(x => Object.keys(this.conditions).some(y => x === y));
+       legacy = checkForExistence.length === 0 ? "" : checkForExistence;
        if (legacy)
        {
            this.disabled = new Set();
            this.active = new Set();

reproducible

Issue can be reproduced in A25 [25848], but not in A24 [24937].

bisect

  • [25077] would only trigger errors, but still allow interaction with the game setup.

  • [25099] would completely prevent interaction with the game setup

related chat

# notes
[13:00:09] elexis try wraitii and my gamesetup rewrite
[13:00:21] Langbart https://code.wildfiregames.com/D3243 ?
[...]
[13:02:01] elexis it also introduced a lot of "legacy" keywords, which inherently sounds like adding deprecated code
[13:02:56] elexis the idea of my gamesetup code was that the JSON file is loaded, if its not valid JSON it might or might not show an error message but then not do any further errors.
[13:02:56] elexis if it is valid JSON it hands over the object to all of the setting parser, and each one takes what is valid or doesn't take anything
[13:03:36] elexis but for that process to never break it would require every single setting parser to never break when receiving a valid JSON with bad semantics (according to the revision of the code)
[13:04:25] elexis so I guess it can be any commit changing any victory setting parser
[13:05:01] elexis I dont know what that legacy code parsing should do, but its usually simpler and more safe to discard an illegal object instead of trying to repair it
[13:05:29] elexis so I suspect its better to delete more code than to extend it
[...]
[13:11:02] elexis SetLegacySetting sounds a lot like supporting to parse old matchsettings
[13:12:47] elexis but the fromInitAttributes reads like it always uses this path
[13:14:30] elexis perhaps he deems the object structure legacy but didnt add code for a new one and thus just calls the old one legacy?

Attachments (2)

25077.jpg (245.6 KB ) - added by Langbart 4 months ago.
25099.jpg (205.9 KB ) - added by Langbart 4 months ago.

Download all attachments as: .zip

Change History (6)

comment:1 by Langbart, 4 months ago

Description: modified (diff)
Milestone: Alpha 26Alpha 27

move up one Alpha

comment:2 by Langbart, 4 months ago

Description: modified (diff)
Keywords: regression added

bisect and chat

by Langbart, 4 months ago

Attachment: 25077.jpg added

by Langbart, 4 months ago

Attachment: 25099.jpg added

comment:3 by wraitii, 4 months ago

Milestone: Alpha 27Alpha 26
Owner: set to wraitii
Patch: Phab:D4678
Priority: Should HaveMust Have

comment:4 by wraitii, 4 months ago

Resolution: fixed
Status: newclosed

In 26933:

Fix gamesetup breakage when matchsettings.json contains incorrect victory conditions.

Reported by: gameboy, langbart

Fixes #6553

Differential Revision: https://code.wildfiregames.com/D4678

Note: See TracTickets for help on using tickets.