Dear List, There was some discussion on the forums about some odd behavior of some of the new filter features in 1.11.x series. Specifically, the filters [allied_with] and [enemy_of] in a standard side filter, and also [filter_vision] in a unit filter or terrain filter.
This was discussed at length on the forums, but in brief there are three issues, in order of increasing significance: 1.) All WML filters in the game (afaik) besides these are designed so that they result in a match whenever *any* object matches the condition. These filters are instead implemented so that *all* objects matching the filter must match the condition, e.g., every side that matches [allied_with] condition, must be allied with the side at hand, in order to match the larger filter. This seems not to have been intended, but it is not necessarily a bug, rather merely an inconsistency with the rest of WML. 2.) Realizing the quirk, but perhaps not realizing the any vs all distinction, it seems that we decided to define exceptional behavior for [allied_with] and [enemy_of] in the case that they do not match any objects -- in this case the subfilter is defined to fail to match, rather than to be vacuously true. So if there are no matches, it behaves like *any*, but if there are multiple matches, it behaves like *all*. I find this fairly confusing / complicated, but again it's debatable whether it is actually a bug. 3.) Finally, [filter_vision] tags are similar in that they require all matching objects to satisfy the condition if there are multiple tags, but very unfortunately, in the case that there are no matches, they do *different* things depending on whether the tag appears in a unit filter or a location filter. For a unit filter, the "tweak" mentioned above is applied, so that the overall filter fails to match if there are no matches to the inner filter, but for a location filter, the tweak is not applied, and the overall filter may succeed when the inner filter has no matches. This I feel is unambiguously a bug and will likely cause great frustration -- it is *not* good for tags with the same name to mean different things. The question now is, what if anything do we want to do in 1.12 branch regarding any of this? (Of course we also should fix in master but how we fix there depends on what we do in 1.12.) We have many options: Regarding [filter_vision] first: - We could do nothing. But, I think (3) is a big enough problem that we should address it somehow. - We could bring the unit_filter version into alignment with the terrain_filter version, by removing the "tweak". - We could change both, so that they match when any side matching the filter has the indicated visibility status. This is my favored option, the reason being, it is the simplest, and I don't think it is actually much more disruptive than the previous option. These filters were introduced in the 1.11.x series, it seems possible that not many people have used them extensively in scenarios yet. My feeling is, if we have to make some kind of change, we might as well just get it like we want it and bury the issue. - We could add a new tag / mode. More on this later. I have made a pull request implementing my favored option here. (However, before it would be merged hypothetically, I would prefer to make a similar change on master and write some unit tests.) https://github.com/wesnoth/wesnoth/pull/237 Regarding [allied_with] / [enemy_of]: - We could do nothing. However, if we are going to make changes to [filter_vision] though, then it is an opportune moment to fix the same problem here also. - We could add a new tag, which implements the *any* functionality which we originally desired. This is my favored option, see the forum discussion for why. I have made a pull request which adds [has_ally] and [has_enemy] which does this. Should we do this, we might also want to add warnings to [allied_with] / [enemy_of] saying "this tag is deprecated...", or we could just leave them, I don't have an opinion about that. https://github.com/wesnoth/wesnoth/pull/238 - We could change the behavior of [allied_with] / [enemy_of]. But since these are actually consistent with eachother, I'm not sure a compatibility breaking change is justified. - Another option was also suggested, that instead of new tags, we could add attributes which modify the behavior, e.g. [allied_with] could have an attribute "require_all" which defaults to yes, but if no is given, then it follows the expected 'any match' behavior. This is not my solution of choice, I think it becomes harder to read. Compare: [has_ally] [has_unit] type=Orcish Grunt [/has_unit] [/has_ally] versus [allied_with] require_all=no [has_unit] type=Orcish Grunt [/has_unit] [/allied_with] IMO it's just much better to have crisp tags that mean what we want, rather than something like [foo] disable_bugs=yes [bar] ... [/bar] [/foo] With that syntax it goes without saying that we won't deprecate later. In summary, my view is that: - For any misbehaving tag, we should provide an alternative without breaking compatibility. - For any truly broken tag (inconsistent behavior), we should just fix it the way it's supposed to be without preserving compatibility with the bug, esp. if the tag is new to 1.11.x series. I find it regrettable to lobby for changes like this to the WML API this close to release, but it seems we don't have great options. Next time we will have some unit tests to catch these problems, and the changes I propose would be accompanied with tests in this cycle. Anyways please write with your views. Best Regards, Chris Beck
_______________________________________________ Wesnoth-dev mailing list Wesnoth-dev@gna.org https://mail.gna.org/listinfo/wesnoth-dev
