Add consistency checks for dangling references#85591
Add consistency checks for dangling references#85591dumb-kevin wants to merge 1 commit intoCleverRaven:masterfrom
Conversation
db58d2c to
2c2bfb5
Compare
|
is looks_like needs to be validated? i thought it was used to target id of a texture, ie if we have item foo, which id was renamed to bar, but tileset still used foo, it would correctly pick the texture named "foo"? or something like that? tho it is very questionable pattern, if it is true The rest LGTM |
dca535f to
98292f4
Compare
9b664aa to
be156c6
Compare
be156c6 to
19c1d9a
Compare
There was a problem hiding this comment.
Auto-requesting reviews from non-collaborators: @Hymore246
19c1d9a to
2093a41
Compare
There was a problem hiding this comment.
Auto-requesting reviews from non-collaborators: @Light-Wave
a3b16ab to
6cf7ca7
Compare
There was a problem hiding this comment.
Auto-requesting reviews from non-collaborators: @ZhilkinSerg
6cf7ca7 to
9095dd8
Compare
9095dd8 to
74511ee
Compare
74511ee to
d152b49
Compare
d152b49 to
3727b75
Compare
| "terrain": [ "afs_crashed_escape_pod" ] | ||
| }, | ||
| { | ||
| "type": "start_location", |
There was a problem hiding this comment.
This should be a real start location. Is the issue it shouldn't be a safe location?
There was a problem hiding this comment.
went and answered my own question. I didn't make it a terrain because I made it a map update. This makes sense.
There was a problem hiding this comment.
oh also apparently I never submitted my review so that's why you never responded to the question.
There was a problem hiding this comment.
So this is resolved, right?
Validate EOC string references, item repairs_like / source_monster / cooks_like / smoking_result / rot_spawn, summon spells with zero duration, profession CBM-without-interface, and start_location om_terrain references. Fix looks_like pointing at removed power_armor_basic / knife_butcher, and use PREFIX match for urban_library variants. Add recipe cycle test for craft/uncraft exploits.
3727b75 to
26ad76b
Compare
Summary
Infrastructure "Add consistency checks for dangling references and fix existing ones"
Purpose of change
Various JSON fields that reference other game entities (item IDs, overmap terrains, EOCs, monster types) were not validated during
check_consistency(), so typos or stale references after renames would silently break things at runtime. A few such broken references already existed.Describe the solution
New C++ checks:
effect_on_condition.cpp- collect EOC string references duringload_inline_eoc, validate they all resolve to real IDs incheck_consistency(). Error messages include both the mod source and the JSON file path for easy lookup.item_factory.cpp- validaterepairs_like,source_monster,cooks_like,smoking_result,rot_spawnmonster/groupmagic.cpp- warn on summon/spawn_item/summon_monster spells with zero duration and no PERMANENT flagprofession.cpp- non-hobby professions with starting CBMs must have a trait that cancelsNO_CBM_INSTALLATION(e.g.CBM_Interface)start_location.cpp- validateom_terrainreferences exist for all match types (exact, type, subtype, prefix, contains)JSON fixes (items):
looks_likepointing at removedpower_armor_basic->combat_exoskeleton_medium(3 files: combat_exoskeleton.json, TEST_DATA/items.json, aftershock exosuit_frame.json)looks_likepointing at removedknife_butcher->knife_huge(cooking.json)smoking_result: " "hack onmutflesh-> proper"null"(carnivore.json)JSON fixes (start locations):
urban_librarychanged to PREFIX match to pick upurban_library_*variantshouse_two_story_basement->house_2story_basement(typo in rename), road entries consolidated to TYPE matchsloc_crashed_gunship- the terrainafs_crashed_gunshipnever existed as an overmap terrain (the gunship is a map extra,mx_gunship; the scenario uses that, not an omt)JSON fixes (EOC dangling references):
EOC_PORTAL_NULL_AWAKENING->EOC_MOM_NO_EFFECT(3 places in eoc_selector menus)EOC_perk_blood_drinking_heal_hp_checkwas a cross-mod dependency on BombasticPerks. Replaced withmod_interactions/bombastic_perks/bridge pattern - no-op base EOC when BombasticPerks isn't loaded, delegates to the real one when it is.EOC_REDUCE_VAMPVIRUS1andEOC_REDUCE_VAMPVIRUS2fromEOC_BLOOD_OF_SAINTS_AGAINST_VAMPIRISMcaller. These were merged intoEOC_REDUCE_VAMPVIRUSin [ Xedra Evolved ] Vampire cure is broken once again #78774 but the caller wasn't updated. Also fixedshade_hands->gracken_shade_hands.EOC_on_kill_check_expetc.) replaced with bridge pattern usingmod_interactions/overrides. Base mod defines no-op stubs,mod_interactions/{bombastic_perks,perk_melee_system,sorcerer}/files override when those mods are active.EOC_SUMMON_MUSHROOMSdefinition (was referenced but never defined)JSON fixes (spells):
PERMANENT_ALL_LEVELSto Magiclysm Alchemist summon spell (zero duration without it)PERMANENTto MoM electrokinesisspawn_itemspell and two Xedraspawn_itemspells (Cultivate Goblin Fruit, Seed Bearer)JSON fixes (professions):
CBM_Interfaceto MMA cyborg martial artist and MoM Fifth Sun profession (had starting CBMs but no interface trait)Build script fix:
get_all_mods.py: useadd_mods()instead of rawappend()when collecting individual mods. The old code skipped dependency resolution and compatibility checks, so mods could be tested without their prerequisites.Other:
flexbuffer_json.h: exposeget_root_source_path()onJsonValue(was protected, needed for EOC error context)New test:
recipe_cycle_test.cpp- checks for net-positive craft/uncraft exploits. Two cases: with-provenance (player-crafted items, checks uniform component groups) and without-provenance (world-spawned items, checks front()-based disassembly path).Describe alternatives you've considered
For cross-mod EOC dependencies (Innawood, MoM psychic vampire), the alternative was to keep inline
mod_is_loadedconditions, but that means the referenced EOC string fails validation when the other mod isn't loaded. Themod_interactions/bridge pattern is already used elsewhere in the codebase and avoids the dangling reference entirely.For
looks_likevalidation -- currently not enforced. There are ~52 unique broken itemlooks_likevalues (27 truly missing IDs, 25 cross-type references like items pointing at furniture). Enforcement is feasible but needs a separate cleanup pass, especially for the cross-type cases that are an intentional pattern for appliances.Testing
make TILES=1): clean[recipe]test tag: 7060 assertions in 5 test cases, all passAdditional context
The
mod_interactions/directory loading mechanism (ininit.cpp) only loads subdirectory contents when the named mod is also active in the world, so bridge EOCs correctly resolve at load time without hard mod dependencies.