General discussion about Era and Modification compatibility

[Ravana]

Only solution for [set_menu_item] that I can see is automatic grouping if there is more than possible to show. Some abilities and weapons specials create menus yet usually no more than 4 for one unit, but does that means I should put 20 for era value as that is about the amount you can get with existing ones within era.

[tekelili]

Some abilities and weapons specials create menus yet usually no more than 4 for one unit, but does that means I should put 20 for era value as that is about the amount you can get with existing ones within era.

You should put the value your era requires. Only era creator can know how many menu_items need to "be reserved".
As I see the issue, menu_items are a lacking resource and creators should know how many they could be claiming with their code.

Edit: maybe my idea is understood better if we name key

Code: Select all

menu_items_reserved=

[Anonymissimus]

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.

The existing automatically stored variables already work like that - the "scoped wml variables".
Say, there's an event callstack with an outer "main" and an inner event, and both have the automatic variable "unit". The inner event at first overwrites the outer variable, but it is restored once the inner event has run out, and once the outer event has run out, the variable is cleared. So, use automatic variables as much as possible and there's no need to worry about overwriting something (with the exception that there shouldn't be manual variables with those names used by automatic variables IIRC, they may get lost) or needing to clear such variables.

I seem to remember that there was a change to the menu item count limit ?! IIRC the reason why there was a limit of 7 were low resolutions.

[Sapient]

I added that limit of seven from the beginning when I first coded the menu_item code. Although I didn't know it would be so controversial, I just figured hey you've got to draw the limit somewhere, and I was thinking ahead to portable devices where you can't assume you have a huge screen.

[tekelili]

I added that limit of seven from the beginning when I first coded the menu_item code. Although I didn't know it would be so controversial,

Maybe I didnt explain myslef with my poor English. I assume there is a good reason for the limit. I am just saying this could easyly happens:
Coder A creates an scenario that can have deffined 6 menu items at same time
Coder B creates a modification that uses 1 menu item
Coder C creates other modification with 1 menu item.
User X runs scenario A + mdifications B and C and have unexpected behavior

With my idea, Wesnoth Engine would have advised user X about limit being violated before scenario being loaded.

[iceiceice]

Or, in case of more than 7 menu items, put a vertical slider on the context menu?

[Sapient]

This may be a personal preference thing, but I'd rather have a choice like "More Menu items" rather than trying to manipulate a slider after a right-click. Such a choice could be added automatically when the screen is out of space to show the menu items.

I'm probably not going to code it myself, but I would approve of such a solution.

[Pentarctagon]

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.

It would depend on where the code is located. A macro is basically just a giant text substitution, so if add-on A called a macro from add-on B (or core), it would be add-on A modifying the variable since the full code would have been placed into add-on A by the preprocessor. Events on the other hand behave (as I understand it) more like functions. So if add-on A were to call an event from add-on B, it would be like add-on B were modifying the variable since the code was not placed into add-on A.

[iceiceice]

This may be a personal preference thing, but I'd rather have a choice like "More Menu items" rather than trying to manipulate a slider after a right-click. Such a choice could be added automatically when the screen is out of space to show the menu items.

I'm probably not going to code it myself, but I would approve of such a solution.

Wait so how would that work? What does the "more menu items" do? Does it launch a window with a slider?

It would depend on where the code is located. A macro is basically just a giant text substitution, so if add-on A called a macro from add-on B (or core), it would be add-on A modifying the variable since the full code would have been placed into add-on A by the preprocessor. Events on the other hand behave (as I understand it) more like functions. So if add-on A were to call an event from add-on B, it would be like add-on B were modifying the variable since the code was not placed into add-on A.

Yeah so I guess this might not require to rewrite the preprocessor, but it would still be tricky, and it still has the problem that if a macro is shared by two different add-ons you will get errors reported.

[Pentarctagon]

It would depend on where the code is located. A macro is basically just a giant text substitution, so if add-on A called a macro from add-on B (or core), it would be add-on A modifying the variable since the full code would have been placed into add-on A by the preprocessor. Events on the other hand behave (as I understand it) more like functions. So if add-on A were to call an event from add-on B, it would be like add-on B were modifying the variable since the code was not placed into add-on A.

Yeah so I guess this might not require to rewrite the preprocessor, but it would still be tricky, and it still has the problem that if a macro is shared by two different add-ons you will get errors reported.

Honestly, that doesn't seem like a problem. Errors should be reported when there are conflicts.

As for the trickiness part, that was why I asked if it would be a good GSoC idea

[iceiceice]

Honestly, that doesn't seem like a problem. Errors should be reported when there are conflicts.

Except when they don't represent an error. This would mean that if there are two add-ons loaded that use the same core macro, we will get errors reported because of the internal variables being overwritten. That's typically not what we want. Macros may not be "subroutines" in a technical sense but most of core and many add-ons basically use them like this.

I think to have a proper notion of ownership and who "should" or "shoudln't" be modifying things, you would have to add proper support for this into the language itself and rewrite core / many of the add-ons to use the new syntax. I don't think it can work properly just with some extra C++ code bolted onto the current setup.

[Pentarctagon]

Honestly, that doesn't seem like a problem. Errors should be reported when there are conflicts.

Except when they don't represent an error. This would mean that if there are two add-ons loaded that use the same core macro, we will get errors reported because of the internal variables being overwritten. That's typically not what we want.

I think to have a proper notion of ownership and who "should" or "shoudln't" be modifying things, you would have to add proper support for this into the language itself and rewrite core / many of the add-ons to use the new syntax. I don't think it can work properly just with some extra C++ code bolted onto the current setup.

Don't core macros with internal variables clear them at the end? If not, or if [clear_variable] isn't sufficient, that could be a prime use case for having local variables.

[tekelili]

Btw Pentarctagon remembered something worried me for years: REPEAT macro.
At this moment I guess is impossible replace it, but I also think is a very bad idea have it included withouth any warning and even recomended in wiki. From my point of view its use is extremly dangerous as if become nested REPEAT_i will be changed in an unexpected way for the coder and will probably lead to infinite loops. This danger is even greater when users start to use several modifications.

My suggestion is:
Add a warning in REPEAT macro (something like #dont use this macro, is deprecated).
Create a macro with explicit index for the same propose that become closed with NEXT macro. Something like this

Code: Select all

#define REPEAT_IT NUMBER VAR
{VARIABLE {VAR} 0}
[while]
    [variable]
        name={VAR}
        less_than={NUMBER}
    [/variable]
    [do]
#enddef

[iceiceice]

Pentarctagon:

Yeah I would tend to agree, if all the core macros clear their variables then it probably is enough for what you want. But it would be hard to guarantee that this is happening, since it never generated warnings before if WML didn't do this.

One thing that you pretty much won't be able to deal with is, what if the macro is supposed to store a result in a variable somewhere? Idk if that can be distinguished from, "overwriting a needed result" each time it is used. But I also don't know if there are many macros that do that, in practice. I'm not sure if there are core macros that do that for instance.



Thinking about it more though, I think what would be more useful for debugging, esp. would be to have an "events debugger". When active, events do not fire instantly, instead the name of the event and the id of the next handler to be fired is displayed, and WML execution put on hold. The user can review the game state in the inspector, then type ":next" to fire the next handler of this type, etc. Could imagine a ":next until" command which fires until an event type / event id occurs. I guess a fancy version would include an "events inspector" gui but IMO that is just icing on the cake. The real problem is when lots of events are cascading / happening too quickly, also when things happen during prestart and you don't even get a chance to see them. It would help a lot with scenarios made of several add-ons also I think, as typically the events of each will still be separate.

I don't think this would be too terribly hard to write, there would just need to be an alternate version of the game_events pump class. But getting the interface right to issue commands to it would be tricky, and also figuring out how to dump info about the current events in a way that is useful, if that is thought to be desirable.

[Pentarctagon]

Pentarctagon:

Yeah I would tend to agree, if all the core macros clear their variables then it probably is enough for what you want. But it would be hard to guarantee that this is happening, since it never generated warnings before if WML didn't do this.

One thing that you pretty much won't be able to deal with is, what if the macro is supposed to store a result in a variable somewhere? Idk if that can be distinguished from, "overwriting a needed result" each time it is used. But I also don't know if there are many macros that do that, in practice. I'm not sure if there are core macros that do that for instance.

Yeah, I have no idea if this is the case either. It wouldn't really be that hard to check, it would just take a fair amount of time and reading.

If a variable is supposed to be overwritten and shared between add-ons, that would be one to put in the [shared_variables] tag.

Thinking about it more though, I think what would be more useful for debugging, esp. would be to have an "events debugger". When active, events do not fire instantly, instead the name of the event and the id of the next handler to be fired is displayed, and WML execution put on hold. The user can review the game state in the inspector, then type ":next" to fire the next handler of this type, etc. Could imagine a ":next until" command which fires until an event type / event id occurs. I guess a fancy version would include an "events inspector" gui but IMO that is just icing on the cake. The real problem is when lots of events are cascading / happening too quickly, also when things happen during prestart and you don't even get a chance to see them. It would help a lot with scenarios made of several add-ons also I think, as typically the events of each will still be separate.

I don't think this would be too terribly hard to write, there would just need to be an alternate version of the game_events pump class. But getting the interface right to issue commands to it would be tricky, and also figuring out how to dump info about the current events in a way that is useful, if that is thought to be desirable.

That sounds really neat. One solution for the prestart/preload/etc events would be to simply dump the info that would have been displayed into an event_log file of some sort. For the cascading events, would it be possible to display the sub-events as a tree? :next could then advance through all the sub-events, showing the cumulative state as the sub-event fire.