Vic3 modifier validation

[BrknRobot]

Incomplete requirements for modifiers:

Every engine static and dynamic modifier needs a modifier_type_definition entry in order to be applied by the game engine.
This includes dynamic modifiers for fully vanilla types. A somewhat common mistake is attempting to use a "vanilla" dynamic modifier that lacks a modifier type definition because PDX only added definitions and localization for the the dynamic modifier variants they used in vanilla content.
eg: goods_input_luxury_clothes_add looks like a 100% vanilla modifier, but vanilla doesn't have a type definition for it, so using it without adding a type definition to your mod is an error.

The prefix of the modifier type definition determines it's type, and propagation. Modifier types with an invalid prefix will generate a error.log entry, but will still appear in game, and can be used to display custom effects implemented elsewhere in script, so should be considered valid
(Tiger might not need to maintain a hard coded map of modifier names and types if it can just use the prefix)

Dynamic building modifiers don't have a prefix, they just look like they do because all the vanilla building definitions start with building_ if you name your building super_factory the modifiers for it look like super_factory_throughput_add not building_super_factory_throughput_add

Applying modifiers in places where they won't propagate to something they will apply to does nothing (and I'm 90% sure there's no error.log message about it, it just silently doesn't work).
eg: A modifier applied to a state which includes country type modifiers will do nothing.
It may be possible to validate modifier propagation to warn if a modifier won't do anything if we can figure out the structure of the modifier graph. I have a hunch that if you can draw a path from one scope to another following scope level iterators, the modifier will propagate.
ie: state type modifiers work when applied to power blocs because you can navigate from power_bloc scope to state scope with every_power_bloc_member -> every_scope_state

Modifier type definitions must be lowercase.
Matching modifier type definitions to the things they apply to is case sensitive. Even if the thing it applies to accepts upper case characters in the definition.

• Building definitions do not need to be lower case, but if you create a building with uppercase characters, you can not use it's dynamic modifiers as the type definitions are invalid.
• Renaming things to remove uppercase characters is a save incompatible change.

Because of this, using uppercase characters in the definition of anything that could later have a modifier type definition created for it is a code smell.

• Fatal error for missing engine static modifier type definitions
• Error for other missing modifier type definitions (Vic3 modifier validation improvements #254)
• Error for uppercase in modifier type definition names (Vic3 modifier validation improvements #254)
• Warn when applying modifiers in definitions that won't propagate (feat(Vic3): Verify flow of modifiers to where they apply #256)
• Warn for using add_modifier in a scope where the contained modifier types won't propagate (feat(vic3): Validate modifier flow with add_modifier #263)
• Warn for using add_enactment_modifier with a static modifier containing modifier types that won't propagate from country scope (feat(vic3): Validate modifier flow with add_modifier #263)
• Derive modifier scope from prefix for script only modifier type definitions (feat(vic3): Validate modifier flow with add_modifier #263)
• Warn for using upper case in item definitions that can have dynamic modifiers generated for them (Vic3 modifier validation improvements #254)

[MasterOfGrey]

Applying modifiers in places where they won't propagate to something they will apply to does nothing (and I'm 90% sure there's no error.log message about it, it just silently doesn't work).
eg: A modifier applied to a state which includes country type modifiers will do nothing.

Correct, notably propagation is strictly downwards, not upwards. Country (OR State Region)>State>Building>Pop

[MasterOfGrey]

Static modifiers defined in common/static_modifiers don't need a modifier_type_definition if they are not intended to propagate through the modifier graph. (The component modifiers still propagate, so I'm not totally sure if there's any practical difference between propagating the top level modifier vs it's components. Maybe something to do with how they're displayed?)

Static modifiers in common/static_modifiers are basically just boxes that contain modifier components. Labelled collections of modifier components with names and graphics.
The components are, in essence, the only actual modifiers at play.

[BrknRobot]

Okay, did some more testing. Modifiers defined in common/static_modifiers have no relation to static modifiers that come from the game engine, and can't be used interchangeably. This was to some extent expected, but it was worth checking given the naming overlap.

[BrknRobot]

Game engine static modifiers do need a modifier type definition. In fact, the game crashes before hitting the title screen if one is missing. Because of this, I think tiger doesn't actually need to be maintaining any list of modifier names. It can just parse the modifier type definitions, and derive the type from the prefix.

[amtep]

Because of this, I think tiger doesn't actually need to be maintaining any list of modifier names

Doesn't a crash make it even more important to verify these modifier type definitions are present?

[amtep]

Dynamic building modifiers don't have a prefix, they just look like they do because all the vanilla building definitions start with building_ if you name your building super_factory the modifiers for it look like super_factory_throughput_add not building_super_factory_throughput_add

This makes me wonder if super_factory_throughput_add would even work, since it lacks the building_ prefix :) Like, even though that prefix comes from the building name, the engine might still require it and respond to it.

I remember there was a dev diary where they discussed modifiers and how they switched to the prefix system for performance.

[BrknRobot]

I was thinking that it's safe to assume the definition exists because PDX wouldn't ship a version that doesn't even load, but I forgot that a mod could overwrite the vanilla file and leave them out. So you're right.

[amtep]

I was thinking that it's safe to assume the definition exists because PDX wouldn't ship a version that doesn't even load, but I forgot that a mod could overwrite the vanilla file and leave them out. So you're right.

Yeah and it can easily happen, if the mod copies the vanilla file and then Pdx adds a new one in the next update.

Not sure why a mod would overwrite the vanilla file, but modders do a lot of things :)

It's good to know that a missing modifier type definition should be logged at fatal severity for these modifiers though.

[kaiser-chris]

I was thinking that it's safe to assume the definition exists because PDX wouldn't ship a version that doesn't even load, but I forgot that a mod could overwrite the vanilla file and leave them out. So you're right.

Yeah and it can easily happen, if the mod copies the vanilla file and then Pdx adds a new one in the next update.

Not sure why a mod would overwrite the vanilla file, but modders do a lot of things :)

It's good to know that a missing modifier type definition should be logged at fatal severity for these modifiers though.

maybe you should add a general untidy message for overwriting a base game file fully?

[BrknRobot]

This makes me wonder if super_factory_throughput_add would even work, since it lacks the building_ prefix :) Like, even though that prefix comes from the building name, the engine might still require it and respond to it.

They do work actually. I think the engine is treating the building name as the prefix, since the way these modifiers work is they propagate down to only that building type, whereas other building_ prefixed modifiers propagate to all buildings

[BrknRobot]

I've updated the issue with refined rules for modifiers and added a checklist of possible improvements

[amtep]

Thanks :)

I would add that there should be an info text added to reports where the problem is that the modifier won't propagate (so: missing type definition, wrong prefix for use case). This will help modders who are trying to figure out why their modifiers don't work.

Also an error message for modifier type definitions that contain uppercase, since they're not going to work.

[MasterOfGrey]

This makes me wonder if super_factory_throughput_add would even work, since it lacks the building_ prefix :) Like, even though that prefix comes from the building name, the engine might still require it and respond to it.

I have a building called usu_building_public_green

and this works:

usu_building_public_green_throughput_add={
	decimals=0
	color=good
	percent=yes
	game_data={
		ai_value=0
	}
}

[amtep]

maybe you should add a general untidy message for overwriting a base game file fully?

I think there are too many valid use cases, especially in total conversion mods, for that to be practical