Suggestion: keep backwards compatibility

Discussion among members of the development team.

Moderators: Forum Moderators, Developers

User avatar
zookeeper
WML Wizard
Posts: 9726
Joined: September 11th, 2004, 10:40 pm
Location: Finland

Suggestion: keep backwards compatibility

Post by zookeeper » March 25th, 2014, 8:42 am

Preamble: as far as I can tell, one of the most annoying, inconvenient and outright damaging things to the UMC community is how the WML API tends to break between stable versions. Add-ons which worked in 1.8 won't work in 1.10, and likely add-ons which worked in 1.10 won't work in 1.12; terrain strings change, tags, keys and macros get renamed or removed or their behavior altered, and so on. What this seems to mean in practise is that a lot of perfectly good add-ons get abandoned because they have no active maintainer to perform all the necessary work to make it work properly on the new stable version. Authors who are active and in the loop might find it no big deal to port their add-ons, but those who aren't can't just drop by every now and then and upload their add-ons to the new server, but instead have to perform all sorts of conversion tasks and thus easily get tired of the whole thing.

My suggestion: as of 1.12, I suggest that the WML API be kept backwards-compatible whenever possible (within reason). If compatibility cannot be maintained even with a bit of effort, then sure it can be broken, but we should absolutely do away with the tendency to break things for example just because we want a tag name to have an underscore now. Looking at past compatibility-breaking changes, it seems that in many many cases there'd have been essentially no difficulty nor added code complexity involved in retaining support for the old behavior and still it wasn't done.

We have wmllint which is supposed to exist exactly for this purpose and which is presumably the justification for compatibility-breaking changes left and right, but the fact is that many people just don't use it because it's not trivial to use, and possibly also because it makes mistakes and produces tons of false positives unless you specifically adapt your code to be wmllint-compatible, which is really backwards. If someone makes it usable even by the average Windows-based UMC author, then my suggestion is moot, but until then "but there's wmllint" isn't much of an argument.

User avatar
Wintermute
Inactive Developer
Posts: 840
Joined: March 23rd, 2006, 10:28 pm
Location: On IRC as "happygrue" at: #wesnoth-mp

Re: Suggestion: keep backwards compatibility

Post by Wintermute » March 25th, 2014, 1:07 pm

I think this is a very good idea. I have listened to many a UMC author wring their hands over this issue and I really don't have an answer for them. We can wave hands and talk about "progress" - and sometimes people really are excited to use the new features that get added - but it still doesn't make it any less time consuming to fix everything. And as pointed out, sometimes really good stuff is lost.

The other factor that this will help address is the backlash from frustrated users who realize that the stable version that they were excited to try out 5 minutes ago doesn't have any of their three favorite add-ons available. I believe many users (and authors alike!) would be greatly relieved if we could keep better backwards compatibility.
"I just started playing this game a few days ago, and I already see some balance issues."

User avatar
jb
Multiplayer Contributor
Posts: 485
Joined: February 17th, 2006, 6:26 pm
Location: Chicago

Re: Suggestion: keep backwards compatibility

Post by jb » March 26th, 2014, 11:08 pm

Hear hear!

I couldn't agree more.
My MP campaigns
Gobowars
The Altaz Mariners - with Bob the Mighty

Max
Posts: 1449
Joined: April 13th, 2008, 12:41 am

Re: Suggestion: keep backwards compatibility

Post by Max » March 27th, 2014, 7:19 am

what if there was only one add-on store and add-ons were tagged with what versions they are compatible?

the add-on manager would get another filter option for older versions. the stable release would only show the compatible add-ons per default, but the dev release would automatically list all add-ons of the last stable version.

my guess and hope would be that this should really help to detecting compatibility issues.

the downside is that umc-devs might (unintentionally) break compatibility with older versions.

User avatar
pauxlo
Posts: 1026
Joined: September 19th, 2006, 8:54 pm

Re: Suggestion: keep backwards compatibility

Post by pauxlo » March 27th, 2014, 5:02 pm

@Max: do you want to auto-detect to which releases an addon is compatible?
If you let the author check, it is in most cases either just "the version I tested" or an optimistic estimation which might not hold.

User avatar
iceiceice
Developer
Posts: 1056
Joined: August 23rd, 2013, 2:10 am

Re: Suggestion: keep backwards compatibility

Post by iceiceice » March 27th, 2014, 10:16 pm

Besides the devs making promises "never again", one thing which was touched on briefly on irc recently is that maybe there are some WML / lua test cases which we want to incorporate into our automatic test suite. (I'm specifically thinking of the lua tests that gfgtdf and EliDupree exchanged regarding synchronize_choice.)

The idea would be that besides having unit tests for certain C++ classes and features, we would also want to do some "end to end" testing. There would be some very simple WML / lua test scenario / replay which we would load and try to play to completion, and if it works the test passes, otherwise it's a failure. The advantage of having it automated is that if a dev accidentally breaks something in the future, travis may report it to them. And if we find in the future that some "core" WML construction broke, we can add it as a test and use git to automatically pinpoint the commit that broke it, which would make such a thing much easier to fix. (Edit: Maybe this is the wrong term, I guess it's not "end to end" so much as "unit testing the WML / lua API".)

I hesitate to propose this, first of all, because I'm violating the KISS rule to some extent -- I'm not completely sure at the moment how to implement this, or what might be the right direction, although I have some ideas. Does anyone reading have any insight into this aspect?
Spoiler:
Second of all it might not be such a good idea because it's not completely clear to me how stable the reloading / replay code is, or if these tests would have to be in a special format to guarantee compatibility with future versions... so it's a bit of a chicken / egg problem :|

However one thing is fairly clear to me anyways. Wesnoth may be a mature project but new features are added all the time and development is still quite active. At the same time there are some core pieces of infrastructure which are implemented in a somewhat unfortunate way and need to be changed in the future (and not just for pedantic reasons, for reasons like preventing segfaults and crashes). If the people that originally wrote these go on Wesbreak and we don't have any unit tests or other kinds of tests for their code... and most of the UMC authors don't test anything until the very end of the beta... then there's not any realistic way that a later developer can perfectly maintain backwards compatibility.

I realize that this isn't what you were talking about (zookeeper) in the OP about suddenly requiring underscores and such, but I think it's still relevant.

I'm also not trying to suggest that this wouldn't be a substantial amount of work, but it may save us some grief in the long run. The purpose of the tests is not to be a pain, just to create more solid foundations for UMC / future development.

Edit: I'm also not suggesting that we systematically go back and write unit tests for everything, but that if we can identify key things used frequently in mainline, then add them, and whenever a bug report is submitted and fixed on the bug tracker, consider making one / several test cases related to it and committing them. Also if anyone proposes that we commit a test for some construction they rely on in their add-on, we can review it and potentially do so. Hopefully if we do this for 1.12 beta and going forward, we'll soon cover most of the stuff that people care about.

Edit: I thought about it some more and made a very simple pull request here: https://github.com/wesnoth/wesnoth/pull/135

User avatar
Elvish_Hunter
Developer
Posts: 1371
Joined: September 4th, 2009, 2:39 pm
Location: Lintanir Forest...

Re: Suggestion: keep backwards compatibility

Post by Elvish_Hunter » March 31st, 2014, 2:48 pm

zookeeper wrote:My suggestion: as of 1.12, I suggest that the WML API be kept backwards-compatible whenever possible (within reason). If compatibility cannot be maintained even with a bit of effort, then sure it can be broken, but we should absolutely do away with the tendency to break things for example just because we want a tag name to have an underscore now.
OK, let's proceed with a concrete example. :eng:
In mainline, we have a macro called FLAG_VARIANT. It is designed to work with flags having four frames; but one of our core flags (ragged) has six frames, and as such it doesn't work properly. In my campaigns, I work around the issue with this macro:

Code: Select all

#define TROLL_FLAG
    # wmlscope: start ignoring
    flag=flags/ragged-flag-[1~6].png:150
    flag_icon=flags/ragged-flag-icon.png
    # wmlscope: stop ignoring
#enddef
I was planning to commit a fix in mainline, but how should it be done?
1) modify FLAG_VARIANT to accept a FRAMES argument
Pros: the macro will become more powerful
Cons: backward compatibility broken, needs wmllint update
2) add a FLAG_VARIANT_6 macro
Pros: backward compatibility is preserved, campaign changes reduced to a minimum, may be enforced by wmllint rule
Cons: won't be useful if a flag with 3, 5 or 7 frames is committed or used in UMC
3) create a FLAG_VARIANT_FRAMES macro and modify FLAG_VARIANT to become a wrapper:

Code: Select all

#define FLAG_VARIANT_FRAMES VARIANT FRAMES
...
#define FLAG_VARIANT VARIANT
	{FLAG_VARIANT_FRAMES {VARIANT} 4}
#enddef
Pros: backward compatibility is preserved, works with any number of frames, may be enforced by wmllint rule
Cons: counter-intuitive macro name?
Which one will be the best route in this case?
zookeeper wrote:If someone makes it (wmllint) usable even by the average Windows-based UMC author, then my suggestion is moot, but until then "but there's wmllint" isn't much of an argument.
Yes, but how should it be modified? By developing a GUI for it? If so, we already had one developed in WxPython - I was able to start it only once on Linux, and even then it didn't work correctly :annoyed: . Most people on Windows won't even know how to install Wx (that, as a cherry on top, doesn't work on Python3 - and sooner or later, I suppose that we'll have to move our Python tools to Python3), so Tk may be the only possible choice (it's included in every Python installation on Windows and it doesn't have that Windows 95 look any more).
But even if we have a GUI front-end, what about the output? Some wmllint messages may be at least cryptic, even more so if a user is a WML novice. And when some add-on crashes wmllint? There's also the problem that several false positives simply cannot be removed, because it works with regexps, while Wesnoth has a WML parser: two different technologies, that yield two different results.
Having a simpler wmllint would be nice for everyone, but the question is: how? :hmm:
Current maintainer of these add-ons:
1.14: The Sojournings of Grog, A Rough Life, The White Troll (co-author)
1.12: Children of Dragons, Wesnoth Lua Pack
Active again until mid-October

User avatar
iceiceice
Developer
Posts: 1056
Joined: August 23rd, 2013, 2:10 am

Re: Suggestion: keep backwards compatibility

Post by iceiceice » March 31st, 2014, 3:17 pm

Maybe wmllint could be provided via a web service? This would at least reduce the burden of having to install it. I assume the burden of network traffic would be manageable? This doesn't address how to simplify the output.

If this parsing thing you speak of is a real issue, then conceivably wmllint could be built into the main wesnoth executable by means of a command line switch? I guess we would have to totally reimplement it then... on the plus side it would come packaged with every release, on the minus side I guess it would be hard to fix any bugs in it for any new release until after they become moot :hmm:

User avatar
zookeeper
WML Wizard
Posts: 9726
Joined: September 11th, 2004, 10:40 pm
Location: Finland

Re: Suggestion: keep backwards compatibility

Post by zookeeper » March 31st, 2014, 4:15 pm

Elvish_Hunter wrote:1) modify FLAG_VARIANT to accept a FRAMES argument
Pros: the macro will become more powerful
Cons: backward compatibility broken, needs wmllint update
2) add a FLAG_VARIANT_6 macro
Pros: backward compatibility is preserved, campaign changes reduced to a minimum, may be enforced by wmllint rule
Cons: won't be useful if a flag with 3, 5 or 7 frames is committed or used in UMC
3) create a FLAG_VARIANT_FRAMES macro and modify FLAG_VARIANT to become a wrapper:

Code: Select all

#define FLAG_VARIANT_FRAMES VARIANT FRAMES
...
#define FLAG_VARIANT VARIANT
	{FLAG_VARIANT_FRAMES {VARIANT} 4}
#enddef
Pros: backward compatibility is preserved, works with any number of frames, may be enforced by wmllint rule
Cons: counter-intuitive macro name?
Which one will be the best route in this case?
I'd say 3 is clearly best, although I'd name it FLAG_VARIANT_N. You can't (or shouldn't) really deduce anything from macro names in the first place, so I don't really see a problem with it.

Personally though, I wouldn't want to encode the number of frames in every macro call neither in the macro name nor an argument, so I'd always rather just make my own macro if I had a nonstandard number of frames, regardless of whether it'd merely consist of a call to FLAG_VARIANT_N or not.
Elvish_Hunter wrote:
zookeeper wrote:If someone makes it (wmllint) usable even by the average Windows-based UMC author, then my suggestion is moot, but until then "but there's wmllint" isn't much of an argument.
Yes, but how should it be modified? By developing a GUI for it? If so, we already had one developed in WxPython - I was able to start it only once on Linux, and even then it didn't work correctly :annoyed: . Most people on Windows won't even know how to install Wx (that, as a cherry on top, doesn't work on Python3 - and sooner or later, I suppose that we'll have to move our Python tools to Python3), so Tk may be the only possible choice (it's included in every Python installation on Windows and it doesn't have that Windows 95 look any more).
But even if we have a GUI front-end, what about the output? Some wmllint messages may be at least cryptic, even more so if a user is a WML novice. And when some add-on crashes wmllint? There's also the problem that several false positives simply cannot be removed, because it works with regexps, while Wesnoth has a WML parser: two different technologies, that yield two different results.
Having a simpler wmllint would be nice for everyone, but the question is: how? :hmm:
Well, most importantly it should have a GUI (which you don't need to launch from the command line, too...) and no dependencies that the user needs to deal with (Python).

The interface would mainly require 1) lots of checkboxes to toggle individual switches with helpful tooltips to explain what they do, 2) a way to view diffs of the changes to be made before applying them, and 3) a way to select the directory to process.

User avatar
Elvish_Hunter
Developer
Posts: 1371
Joined: September 4th, 2009, 2:39 pm
Location: Lintanir Forest...

Re: Suggestion: keep backwards compatibility

Post by Elvish_Hunter » April 4th, 2014, 9:43 am

iceiceice wrote:Maybe wmllint could be provided via a web service? This would at least reduce the burden of having to install it. I assume the burden of network traffic would be manageable?
In that case, we'll theoretically need to find a free Python hosting (I suppose that the traffic will be low). Theoretically, because wmllint so far was designed to be run locally, and I have no idea about the required modifications.
iceiceice wrote:on the minus side I guess it would be hard to fix any bugs in it for any new release until after they become moot
To not speak of the fact that it will become harder to fix bugs in general...
zookeeper wrote:I'd say 3 is clearly best, although I'd name it FLAG_VARIANT_N. You can't (or shouldn't) really deduce anything from macro names in the first place, so I don't really see a problem with it.
Thanks :) . I'll handle it as soon as I can - currently, I'm busy with the [elseif] stuff.
zookeeper wrote:Well, most importantly it should have a GUI (which you don't need to launch from the command line, too...)
Otherwise that'll be like the cat (or was it the dog?) that bites its own tail :P . Python is perfectly able to run scripts without launching the command line, as long as its interpreter path is added to the PATH environment variable. Such task is performed when installing it, but it must be explicitly allowed in the setup. I suppose that we'll probably need to add a Installing Python on Windows 101 pdf - or something to that effect - into the tools directory...
zookeeper wrote:no dependencies that the user needs to deal with (Python).
That makes things a lot more complex. It is possible to compile Python programs to exe with cx_Freeze or py2exe, but the problem is that wmllint relies on other Python tools, so I have absolutely no idea how it'll end up - probably in a total bust. :hmm: I already have bad experiences with PyEnchant... :augh:
zookeeper wrote:The interface would mainly require 1) lots of checkboxes to toggle individual switches with helpful tooltips to explain what they do
Tooltips. A widget that isn't available in Tk, unless one creates its own Image.
zookeeper wrote:2) a way to view diffs of the changes to be made before applying them
The best way will be to run the command line wmllint in a separate thread (to avoid freezing the GUI), and then push its text output into a text editor widget - but that's the way to ideally handle every wmllint output.
zookeeper wrote:3) a way to select the directory to process
That's the simplest thing to do: a nice Browse button 8) .
Current maintainer of these add-ons:
1.14: The Sojournings of Grog, A Rough Life, The White Troll (co-author)
1.12: Children of Dragons, Wesnoth Lua Pack
Active again until mid-October

User avatar
Elvish_Hunter
Developer
Posts: 1371
Joined: September 4th, 2009, 2:39 pm
Location: Lintanir Forest...

Re: Suggestion: keep backwards compatibility

Post by Elvish_Hunter » April 12th, 2014, 8:39 am

Update on the wmllint GUI project. So far I coded the main window. I'm not yet ready to disclose the code (I'm still working on it), but what I can show you is a screenshot of it:
wmllint.png
Does it look good enough, or should I modify something?
Current maintainer of these add-ons:
1.14: The Sojournings of Grog, A Rough Life, The White Troll (co-author)
1.12: Children of Dragons, Wesnoth Lua Pack
Active again until mid-October

User avatar
zookeeper
WML Wizard
Posts: 9726
Joined: September 11th, 2004, 10:40 pm
Location: Finland

Re: Suggestion: keep backwards compatibility

Post by zookeeper » April 12th, 2014, 9:18 am

I don't really know wmllint's all switches so I can't say if more of them should be exposed directly, but other than that it seems pretty good. Some random observations/ideas/etc:

1. The wmllint mode and verbosity level options would need some sort of explanation though. If verbosity level really is just a number then I guess that can't be helped, but there seems to be plenty of space for inline explanations for the wmllint mode options. I wouldn't have any idea what "clean" and "revert" do, for instance.

2. There should be a high-level option to specify whether you want it to a) check for image/sound references, b) check for obsolete WML (such as renamed tags or macros) and/or c) perform semantic sanity-checking (valid speakers, translation markers, etc). Checking for broken references and obsolete syntax is a whole different thing than the semantic checks which are much more prone to give tons of false positives, and thus they should IMO be two rather separate modes somehow.

User avatar
Elvish_Hunter
Developer
Posts: 1371
Joined: September 4th, 2009, 2:39 pm
Location: Lintanir Forest...

Re: Suggestion: keep backwards compatibility

Post by Elvish_Hunter » April 12th, 2014, 9:37 am

zookeeper wrote:I don't really know wmllint's all switches so I can't say if more of them should be exposed directly
Here you go:

Code: Select all

Usage: wmllint [options] [dir]
    Convert Battle of Wesnoth WML from older versions to newer ones.
    Also validates WML to check for errors.
    Takes any number of directories as arguments.  Each directory is converted.
    If no directories are specified, acts on the current directory.
    Options may be any of these:
    -h, --help                 Emit this help message and quit.
    -d, --dryrun               List changes but don't perform them.
    -v, --verbose              -v        lists changes.
                               -v -v     names each file before it's processed.
                               -v -v -v  shows verbose parse details.
    -c, --clean                Clean up -bak files.
    -D, --diff                 Display diffs between converted and unconverted
                               files.
    -r, --revert               Revert the conversion from the -bak files.
    -s, --stripcr              Convert DOS-style CR/LF to Unix-style LF.
    -p, --progress             Names each file before processing (like -v -v).
    -f, --future               Enable experimental WML conversions.
    -K, --known                Suppress check for unknown unit types, recruits,
                               races, scenarios, etc.
    -S, --nospellcheck         Suppress spellchecking
    -Z, --stringfreeze         Suppress repair attempts of newlines in messages
For more about wmllint, including how to prevent unwanted conversions and false
positive warnings with magic comments, read the introduction in the wmllint
file itself.  See also: http://wiki.wesnoth.org/Maintenance_tools.
zookeeper wrote:but other than that it seems pretty good
Thanks. 8)
zookeeper wrote:The wmllint mode and verbosity level options would need some sort of explanation though. If verbosity level really is just a number then I guess that can't be helped, but there seems to be plenty of space for inline explanations for the wmllint mode options.
Agreed on this. That was one of the reasons why it's still a work in progress.
zookeeper wrote:I wouldn't have any idea what "clean" and "revert" do, for instance
That can be fixed as well.
zookeeper wrote:a) check for image/sound references
If you're talking about missing resource files, that's wmlscope's job, and that will require a different GUI. Or to create another tab in the same window to control wmlscope - after all, ttk has a tabbed Notebook widget.
zookeeper wrote:b) check for obsolete WML (such as renamed tags or macros) and/or c) perform semantic sanity-checking (valid speakers, translation markers, etc)
Mine will merely be a GUI front-end to make stuff easier; enabling or disabling checks for obsolete WML will require to modify wmllint itself, not my GUI. Not until a new command line switch is added, at least. ;)
Current maintainer of these add-ons:
1.14: The Sojournings of Grog, A Rough Life, The White Troll (co-author)
1.12: Children of Dragons, Wesnoth Lua Pack
Active again until mid-October

User avatar
Elvish_Hunter
Developer
Posts: 1371
Joined: September 4th, 2009, 2:39 pm
Location: Lintanir Forest...

Re: Suggestion: keep backwards compatibility

Post by Elvish_Hunter » April 19th, 2014, 9:28 am

Updates on both tasks.
Today I pushed in master the FLAG_VARIANT6 macro, also taking care of updating wmllint and mainline campaigns:

Code: Select all

[11:06] <irker559> wesnoth: Elvish_Hunter wesnoth:master 9289a9fa8a70 / data/core/macros/image-utils.cfg: added FLAG_VARIANT6 macro, for use with ragged flags http://git.io/3VZDuQ
[11:06] <irker559> wesnoth: Elvish_Hunter wesnoth:master 9e140c183cac / data/tools/wmllint: added wmllint rule for FLAG_VARIANT6 http://git.io/AvqBZA
[11:06] <irker559> wesnoth: Elvish_Hunter wesnoth:master b22ceefa0d4e / data/ (80 files in 17 dirs): Changed all mainline occurrences of {FLAG_VARIANT ragged} to {FLAG_VARIANT6 ragg http://git.io/FKxFZw
[11:06] <irker559> wesnoth: Elvish_Hunter wesnoth:master c2062caea6ab / changelog: Changelog entry http://git.io/cEUlNg
I decided to go for the second version of the macro for consistency with other core macros (like QUANTITY4).
Update on the wmllint GUI project:
wmllint gui.png
This is how it looks on Linux, but only because I haven't yet implemented ttk.Style to give it a more modern look.
As you can see, I added the descriptions suggested by zookeeper (are they good enough?), plus a Clear output button, in case that one want to delete the content of the text box. There are also some things going under the hood, like the fact that the application uses a peculiar technique to run wmllint, partialy inherited by its Wx counterpart:
  • First, build a command line
  • Second, create a thread. Otherwise the application will be frozen until wmllint ends
  • Third, run wmllint as a subprocess, Then, a popup with a progress bar (that doesn't really track the progress, but serves the purpose of telling the user that the script isn't blocked) appears. So far, I haven't yet found a convenient way to stop the wmllint subprocess, so that popup won't have a Stop button
  • Finally, push the wmllint output and errors in the text box.
Finally, a thing that I was thinking to do was importing the gettext library, so that at least this GUI could be translated. Are our mainline tools ready to handle translatable strings in languages other than C++, WML and Lua?
Current maintainer of these add-ons:
1.14: The Sojournings of Grog, A Rough Life, The White Troll (co-author)
1.12: Children of Dragons, Wesnoth Lua Pack
Active again until mid-October

User avatar
vultraz
Community Manager
Posts: 927
Joined: February 7th, 2011, 12:51 pm
Location: Dodging Daleks

Re: Suggestion: keep backwards compatibility

Post by vultraz » April 19th, 2014, 10:53 am

I think you're being too verbose with your labels. For example, you don't need to say "Clean" AND "Delete .bak files". I recommend using only the latter string for all mode labels. Also:

Run wmllimnt -> Run
Select a directory -> Directory
Select wmllint mode -> Mode
Verbosity level -> Verbosity
Set wmllint options -> Options
wmllint output -> Output
Creator of Shadows of Deception (for 1.12) and co-creator of the Era of Chaos (for 1.12/1.13).
SurvivalXtreme rocks!!!
What happens when you get scared half to death...twice?

Post Reply