EOCs don't validate effects inside the eoc until the eoc is run

[Maleclypse]

Describe the bug

On PR #65311 commit 957026e40390ff9b80205ae5f9ffbb758b012cf5 passed github CI all green and the game loaded normally on my local device until I achieved the conditions that activated the following eoc

{
    "type": "effect_on_condition",
    "id": "EOC_ALCHEMY1",
    "condition": { "and": [ { "u_has_trait": "perk_ALCHEMY1" }, { "not": { "u_has_effect": "mental_exhaustion" } } ] },
    "effect": [
      {
        "u_roll_remainder": [ "extra_str_aspirin", "blackout_rage_drink", "sevendust", "small_blessing_brigit" ],
        "type": "recipe"
      },
      { "u_add_effect": "mental_exhaustion", "intensity": 1, "duration": "24 hours" }
    ]
  }

At which point the following errors appeared.

18:51:55.261 ERROR : C:\Users\colli\Documents\GitHub\Cataclysm-DDA\src\effect.cpp:94 [obj] invalid effect type id mental_exhaustion
18:52:07.391 ERROR : C:\Users\colli\Documents\GitHub\Cataclysm-DDA\src\creature.cpp:1368 [add_effect] Invalid effect, ID: mental_exhaustion

Both github actions and at world start validation should have caught this ahead of time.

Attach save file

Test Xedra Perks-trimmed.tar.gz

Steps to reproduce

Compile a world based on the listed commit above. Load into this save game. Gain 100 XP, select Basic Alchemical Knowledge perk, activate said perk in mutations, watch the error fly and the roll_remainder not happen either.

Expected behavior

CI or load validation to catch this.

Screenshots

No response

Versions and configuration

• OS: Windows
  • OS Version: 10.0.22621.1555 (22H2)
• Game Version: 0.G-1612-g957026e403 [64-bit]
• Graphics Version: Tiles
• Game Language: System language []
• Mods loaded: [
  Dark Days Ahead [dda],
  Disable NPC Needs [no_npc_food],
  Portal Storms Ignore NPCs [personal_portal_storms],
  No Fungal Growth [no_fungal_growth],
  Bionic Professions [package_bionic_professions],
  Xedra Evolved [xedra_evolved],
  Bombastic Perks [bombastic_perks]
  ]

Additional context

No response

[andrei8l]

The code supports this syntax too (after the bugfix in #65444)

{ "u_add_effect": { "global_val": "runtime_target_effect" }, "intensity": 1, "duration": "24 hours" }

So the effect type is read from a variable and that's impossible to validate during JSON loading.

[Maleclypse]

The code supports this syntax too (after the bugfix in #65444)

{ "u_add_effect": { "global_val": "runtime_target_effect" }, "intensity": 1, "duration": "24 hours" }

So the effect type is read from a variable and that's impossible to validate during JSON loading.

So should I close this as unsolvable?

[andrei8l]

No, keep it. We should be able to validate the simple case with a string (like in your example) during the Verifying/check_consistency phase.

[Ramza13]

I don't think its that simple. The problem with validating this stuff is that by the verifying check consistency phase we don't have anything other than a function. It would take a lot of work and changes to keep around the original value to verify after the fact. Its one of the tradeoffs that happened when I abandoned my original version of EOCs to use the dialog system. Unless I'm missing something and you have an idea for how to handle this?