General discussion about Era and Modification compatibility

[tekelili]

Hi, I would like offer my opinion but also get advises and opinions about how interactions between umc addons, eras and modifications should be managed.

My main reason for this is I feel right now there is an "unfair" pressure from users on addon creators to be compatible with eras. In general, it is spreat perception that addons should be compatible with any era...

With new keys added in Bfw 1.12 to scenarios and eras to manage what is allowed and introduction of tag [modifications], I think would be a nice moment to create some kind of policy recomended.

I have some ideas:
- It should recomended to era creators to use allow_scenario restricted to those they know will have expected behavior (or at least acceptable and not crash behavior). By not using allow_scenario key, it is like saying "this era will work with any scenario", wich in many cases will be false. Iirc scenario creators have still some ways to allow an era oversighting era settings if neccessary.

- It should be recomended to era creators that dont offer new factions, be recoded as [modifications] in 1.12.

- As there is a limit in [set_menu_item] wml allows, would be nice creation of some kind of standar comunication between add ons to check limit is not being surpassed.

...those were my 2 cents, hoping for feedback

[Velensk]

I think a lot of it would have to be done on a case basis depending on the nature of the add-on though I definately agree that many of the things formerly done as eras would be far more convienient as modifications [for example, No Mirror].

I havn't seen any modifications that'd be a problem with a generic era but I suppose that when you get into the specialized ones for both sides it could be a problem (then again the only modification that I've seen is the PYR modification which works fine with any era).

[zookeeper]

I'd also suggest that when whitelisting all potentially compatible eras/scenarios/modifications isn't possible, one should make some in-game checks in a start event or similar to detect whether at least some minimum requirements for compatibility are met, and if not, show a warning about it to the player(s). Of course that's not always possible or feasible, but in many cases there's likely some sanity-checking one can do if your era/scenario/modification has any particular requirements of the context in which it's used.

[lipk]

- It should recomended to era creators to use allow_scenario restricted to those they know will have expected behavior (or at least acceptable and not crash behavior.

That depends on what kind of era you make. If it's just a bunch of units, there's really nothing that would prevent it from working on most (present and future) maps. so restricting it to scenarios that you tested explicitly is not a good idea. If it's some really special era with many events and heavy dependence on certain map features, then sure, a whitelist is fine.

Iirc scenario creators have still some ways to allow an era oversighting era settings if neccessary.

Yes, there's a key for this, ignore_incompatible_era (/scenario/mod).

[Pentarctagon]

It would be really useful if wesnoth displayed a warning if an add-on is using a variable created by another another add-on.

[Anonymissimus]

It would be really useful if wesnoth displayed a warning if an add-on is using a variable created by another another add-on.

Not possible I guess. We can add to the guidelines that variables should have an addon-prefix. At least if the addon is ment to be compatible with more than a few other addons. The problem is/was already present with macros in multiplayer, and scenario-era interaction too (it just got more severe).

I'd also suggest that when whitelisting all potentially compatible eras/scenarios/modifications isn't possible, one should make some in-game checks in a start event or similar to detect whether at least some minimum requirements for compatibility are met, and if not, show a warning about it to the player(s). Of course that's not always possible or feasible, but in many cases there's likely some sanity-checking one can do if your era/scenario/modification has any particular requirements of the context in which it's used.

Or, to avoid potentially difficult sanity checking, add a description about expected compatibilities and/or conflicts.

It should be recomended to era creators that dont offer new factions, be recoded as [modifications] in 1.12.

More precisely, if an era addon contains any other toplevel tag than [era] and/or custom images/sounds/music, it cannot be a modification (or at least it doesn't make much sense that it is a modification, since IIRC there's also require_modification=). For anything else, containing only events, yes.

[Pentarctagon]

It would be really useful if wesnoth displayed a warning if an add-on is using a variable created by another another add-on.

Not possible I guess. We can add to the guidelines that variables should have an addon-prefix. At least if the addon is ment to be compatible with more than a few other addons. The problem is/was already present with macros in multiplayer, and scenario-era interaction too (it just got more severe).

Would introducing add-on level scope for variables/macros would be a good GSoC project?

[gfgtdf]

@Pentarctagon is really needed? i personaly think using addon-prefixes for variables isn't too hard, i think the current guideline is that eras & modifiations shall use addon-prexixed variables and scenarios should use "normal" variablenames.

Also in some cases it is desired that mods/eras/scenarios interact with each others which would also be harder with add on scoped variables.

[iceiceice]

As I understand, persistent/global variables do have a namespace associated to them. (But I have never tested it, and it doesn't seem to have any unit tests, so who knows if it actually works.)

I agree with gfgtdf, I think it's desirable for add-ons to be able to write over eachothers' variables.

[Pentarctagon]

Mostly I'm just worried about the difficulty of debugging when there's a variable name collision. IIRC, macros can't be defined twice without giving a warning at least in the stderr, but trying to find the source of an error when variables from a scenario, an era, and an unlimited number of modifications can freely and silently overwrite each other sounds like an absolute nightmare.

[Sapient]

You could have the best of both worlds with something like a default scope, differing by add-on, and an ability to override that scope for cross-add-on variables

I am saying this without having actually used the feature myself so not sure how relevant my suggestion may be.

[iceiceice]

So in a way we already have this problem, with unit_type ids. In that case, every add-on which uses a "well-known" unit appearing in other add-ons is supposed to use its own special prefix to prevent collisions. So that problem is somewhat solved already -- add-on makers know that they may need to have their own special prefix to avoid collisions. Using the same for variables really shouldn't be an issue.

One thing which can help problems like this would be if we added a feature to WML similar to the lua "local" statement. The idea is that, if you are just using a temporary variable to store a unit / count through a loop, and you will discard it at the end of the event, you can declare it explicitly as a local variable, so that the engine knows to restore its original value when your event ends, preventing a possible collision with another add-on / another event of yours. Idk if it would be considered worth it though.

Mostly I'm just worried about the difficulty of debugging when there's a variable name collision. IIRC, macros can't be defined twice without giving a warning at least in the stderr, but trying to find the source of an error when variables from a scenario, an era, and an unlimited number of modifications can freely and silently overwrite each other sounds like an absolute nightmare.

The alternative, where we do run time analysis of variable ownership, seems very difficult. For one variables currently aren't "owned" by anyone. And events aren't either. If you save and reload, generally speaking all the events are on the same footing and there's not really a record of which one came from which add-on.

Besides this, WML is complicated enough that deciding "ownership" is not straightforward. Suppose add-on A defines a custom event X, which modifies some variable. Suppose add-on B defines a custom event Y, which fires X. Who owns the variable, X or Y?

Suppose that add-on A is declared as a dependency of add-on B, and also is a dependency of add-on C, and C also has a custom event Z which fires X. If B and C exist in the same scenario and both fire the helper code in A, who owns the variable?

I think hoping for a general automated solution is ... hopeful, but in practice if you just use your prefix, you won't have a collision and this isn't an issue. If you choose not to use your prefix / intentionally modify someone else's variables, that's your decision.

[Pentarctagon]

What about, rather than trying to assign ownership to every variable and keep track of it, keeping track of who modified it last? If the add-on modifying it is different than the add-on that last changed it, put a warning in the stderr. Variables meant to be shared could be explicitly declared through a [shared_variables] tag or something similar. You'd still need to somehow separate/label the code from different add-ons to know who's modifying the variable, but it sounds much easier than trying to sort through dependencies/events/etc.

[iceiceice]

What about, rather than trying to assign ownership to every variable and keep track of it, keeping track of who modified it last? If the add-on modifying it is different than the add-on that last changed it, put a warning in the stderr.

These are substantively the same thing (at least as I am using the terms.)

Variables meant to be shared could be explicitly declared through a [shared_variables] tag or something similar. You'd still need to somehow separate/label the code from different add-ons to know who's modifying the variable, but it sounds much easier than trying to sort through dependencies/events/etc.

That's actually substantively the same as "trying to sort through the dependencies/events/etc." When you actually get to the point of playing a scenario, the game doesn't really have a notion of your add-on or the files it came from anymore, it just has a list of registered event handlers, with associated WML. Either the event handlers themselves or the WML needs to be annotated with ownership info for the system to work. But that ownership info in general doesn't exist, and I think it's ambiguous anyways.

Take a different example. Suppose add-on A defines a macro, that add-on B uses. If the macro sets a variable, who "did it"? Keep in mind that for practical purposes we probably have to say add-on B, or rewrite the WML preprocessor. On the other hand, if the exact same code that was instantiated appeared in A's file, so that A introduced the event, we would probably have to say that A owns it.

We could just throw our hands up and say, if add-on interacts directly with events / macros from another add-on, then we have no idea who owns it and will ignore the ownership info. But I think this means the feature will have limited usefulness in the cases that are the hardest.

Another example: What if your add-on uses a core macro that changes a variable? If the add-on itself "owns" that variable change, that means that if any other add-on tries to use the same core macro, you will get errors reported to stderr, which is almost surely not desired. It means that at most one add-on would "own" and be able to use the core function, effectively. So the only option to avoid that is to rewrite the preprocessor, and annotate all macro expansions with ownership info.

[tekelili]

I saw no comments abot [set_menu_item] limit. From my point of view this should be a common problem when players get used to some [modifications] as "Preset Advancement" or "Dov XP Mod". I think an easy solution would be that tags

Code: Select all

[modification]
[era]
[scenario]

would have an optional key

Code: Select all

menu_items_defined=

and engine could easyly add those values and display a warning message when limit is violated.

It doesnt look to add to much work nor to engine neither to coders.