1.17 terrain-graphics macros clean-up #6606

doofus-01 · 2022-04-03T02:13:29Z

I'll try to clear out the unused and used, but not really needed, terrain-graphics macros, so that migrating to a different graphics engine is less daunting. This will take a while to reach merge state.

In general, I believe many (most?) of the terrain macros are legacy, and no longer really needed. Most terrains can use one of the NEW:* macros, those are the ones to keep and update. The *_PLFB-style and "meta-macro" + auto-generated rules are prime targets for removal, but the NEW macros aren't quite ready to take over as-is.

First thing to tackle is to replace the OVERLAY_* macros

EDIT: and I tried to skip CI from memory, but didn't get it right, apparently.

cooljeanius · 2022-04-03T02:31:01Z

please add wmllint rules to convert any replaced macros with their replacements, for the benefit of UMC that may be using them

doofus-01 · 2022-04-03T02:33:42Z

There's a whole deprecation headache to deal with at the end. Don't worry, I'm not trying to break UMC.

Pentarctagon · 2022-04-03T16:48:39Z

Thanks a bunch for working on this.

doofus-01 · 2022-04-04T02:00:13Z

I can't do much to help with moving to a new engine, but this is the one thing I can do to at least make it more manageable.

I think most of the old macros are unused now, the last thing to deal with is WALL_TRANSITION*.
Then wmlindent, woptipng, etc. and deal with the deprecation, which leads to a question:

Do the terrain macros need to have some formal deprecation process with logged warnings? Or is it enough to move them to a "deprecated" directory? Getting wmllint to automatically change things seems unrealistic. If the old macros still work, but are clearly marked as not relevant for mainline or future content, that should still achieve the goal of getting them out of the way of future development.

Pentarctagon · 2022-04-04T03:13:31Z

The process would just be adding #deprecated to them, I'd think.

doofus-01 · 2022-04-04T04:06:48Z

Well, that doesn't sound so bad. Something to look into next week.

doofus-01 · 2022-04-09T22:32:14Z

What is the right deprecation level for the old macros, can it be level 3 or even 4?

CelticMinstrel · 2022-04-09T22:47:34Z

I tried to skip CI from memory, but didn't get it right, apparently.

I think that was a Travis thing that GitHub Actions doesn't actually support.

What is the right deprecation level for the old macros, can it be level 3 or even 4?

It depends on the intention.

This macro is being phased out but we don't mind if you keep using it, because it'll continue to exist for the foreseeable future.
This macro is being phased out and you should probably stop using it, as it is likely to be removed at some unspecified point in the future.
We plan to remove this macro in the next stable release, so you should stop using it as soon as possible.
Due to unfortunate circumstances (usually a difficulty in implementing a legacy macro in terms of newer macros), we are unable to continue to support this macro, and you must update your code immediately or it will cease to work.

You shouldn't use 4 unless you absolutely have to. If you do use 4, the macro itself would probably either do nothing but contain the deprecation message, or it would do something that only vaguely approximates what it used to do. If the intention is to get rid of these macros as soon as possible, the correct deprecation level is 3.

That said, you should probably check with @Pentarctagon for approval before using level 3.

CelticMinstrel · 2022-04-09T22:50:20Z

data/core/terrain-graphics.cfg

-{OVERLAY_COMPLETE_LF     Ss        (!,Xv,Chs,!,C*,H*,M*,X*,Q*,A*,Ke,Kea,Kud*,I*) -85 base2      swamp/reed}
-{OVERLAY_COMPLETE_LF     Chs       (!,C*,K*,Ss)                              -85 base2      swamp/reed}
+{NEW:OVERLAY              Ss                                                swamp/reed LAYER=-85 FLAG=base2 ADJACENT="!,Xv,Chs,!,C*,H*,M*,X*,Q*,A*,Ke,Kea,Kud*,I*" }
+# {OVERLAY_COMPLETE_LF     Ss        (!,Xv,Chs,!,C*,H*,M*,X*,Q*,A*,Ke,Kea,Kud*,I*) -85 base2      swamp/reed}


The old versions commented out should be removed entirely before this is merged. I realize this is still in draft though, so probably not a priority right now.

I'm leaving a lot of that stuff in for now until deprecation is worked out.

That's fine. As long as it's gone before it's merged.

CelticMinstrel · 2022-04-09T22:53:38Z

data/core/terrain-graphics.cfg

@@ -657,12 +677,17 @@ C*,K*,X*,Q*,W*,Ai,M*,*^V*,*^B*,_off^_usr#enddef
 {NEW:CASTLEWALL             Km            (!,Km,Xu*,Xo*)        K*                castle/aquatic-castle/keep}

 # disable the next line to get the brown bank transition.  It adds more images to the hex though.
-{DISABLE_BASE_TRANSITIONS_F     (Km,Cm)     non_submerged}
-{DISABLE_BASE_TRANSITIONS     (Km,Cm)}
+# I don't remember what these were for, and can't see that they make a difference


Does anyone else know the purpose of these?

data/core/terrain-graphics.cfg

Pentarctagon · 2022-04-09T22:58:11Z

It's probably unlikely for a GUI toolkit replacement to make it into 1.18 (unless maybe if we go with TGUI and it's exceptionally simple to swap in), so I'd say deprecation 2 for now, and then hopefully deprecation 3 or 4 in 1.19.

CelticMinstrel · 2022-04-09T23:00:19Z

I don't quite understand what that has to do with this, but okay, sure.

doofus-01 · 2022-04-09T23:02:09Z

Thanks. 2 it is.

Pentarctagon · 2022-04-09T23:43:37Z

I don't quite understand what that has to do with this, but okay, sure.

From the PR description, one of the motivations of this is to simplify what we'd need to potentially re-implement if we were to end up switching to a different GUI toolkit. At which point any functionality specific to these macros wouldn't need to be redone, and they could just be removed instead.

CelticMinstrel · 2022-04-10T01:24:32Z

Okay… well, if I recall correctly, anything deprecated at level 2 or above can be removed whenever we feel the need to, provided there has been at least one stable series in which it was deprecated. (If it was deprecated at level 1 I believe it would need to be promoted a level for one series before removing it.)

So, it sounds like the correct answer is indeed:

#deprecated 2 1.20 Your Message Here

CelticMinstrel · 2022-04-10T19:16:30Z

data/core/terrain-graphics/deprecated-base.cfg

@@ -1,6 +1,9 @@
 #textdomain wesnoth

 #toplevel macros to handle base tiles
+# These macros are legacy code, the entire file can be retired
+
+#deprecated 2 1.19.0 Any terrain macro with a TERRAIN_BASE_* name pattern is being retired and uses deprecated internal macros


This is not the correct way to deprecate these. It will result in the deprecation message being shown always. You'll need to copy-paste this line into each macro individually.

Basically, this deprecation message is shown when someone includes this file as {core/terrain-graphics/deprecated-base.cfg}, but the core _main.cfg does this, so it will always show.

I know, thanks.

… ends

doofus-01 · 2022-04-11T02:09:52Z

I've tested this in the editor, the test scenario, and a few MP scenarios in case there was anything surprising there, and it seems to work. I plan to merge this if it passes CI and no one has objections over the upcoming week.

This isn't completely optimized, but I think the bulk of the legacy code is contained. New/updated macros start with NEW: and as for the files:

deprecated-*.cfg files contain deprecated macros
enduring-*.cfg files contain old macros that weren't messed with much
new-*.cfg files are updated or were new enough not to need updating

Overlapping (out of hex) images, animation timing, and optional arguments weren't always available, a lot of the old macros were a method of working without those features.

doofus-01 · 2022-04-16T12:00:30Z

@Pentarctagon - According to the roadmap, 1.17.3 is due tomorrow, is it better to merge this before or after? I don't think it will change anything it wasn't supposed to, but I could have missed something, so I'm not sure if it needs to stew in a "1.17.x+dev" version first.

Pentarctagon · 2022-04-16T15:22:40Z

I'd say go ahead and merge it.

* updates to desert terrain * ^Bsa snowy stone bridges * fix wooden bridge bug introduced by #6606

initial OVERLAY revision [skip_ci]

d8c356b

github-actions bot added the Terrain Issues that involve terrain definitions or their implementation in the engine. label Apr 3, 2022

doofus-01 added 2 commits April 3, 2022 08:16

address where trash and gates rules stand in this cleanup

8731f38

more OVERLAY removal, dealing mostly with desert mountains for now

702c4c7

github-actions bot added the Graphics Issues that involve the graphics engine or assets. label Apr 3, 2022

doofus-01 added 4 commits April 3, 2022 12:21

finish up with getting desert mountains off of old macros

7ea2145

update some plant embellishments

b1d6e79

clean out AQUATIC from legacy macros

eebeb8c

conversion of chasm/void transitions

131cec8

minor updates

061c89b

update WALL_TRANSITION

d160002

doofus-01 mentioned this pull request Apr 9, 2022

Convert images to webp format #6611

Merged

CelticMinstrel reviewed Apr 9, 2022

View reviewed changes

first deprecation (level 2) notices

91c0379

updating and deprecation progress, not finished...

e25bf7e

CelticMinstrel reviewed Apr 10, 2022

View reviewed changes

all terrain macro files reviewed, still need cleanup and to tie loose…

68027a8

… ends

doofus-01 added 5 commits April 10, 2022 13:27

update core terrain-graphics config

a55a197

fix a few mistakes, now editor can load again

e69a098

SoF - update terrain macros

aee8a8a

UtBS - update terrain macros

056b39c

UtBS - missed a couple old terrain macros

ef1e8ed

github-actions bot added the Campaign (any) Deprecated tag, replaced with separate tags for each mainline campaign label Apr 10, 2022

doofus-01 added 4 commits April 10, 2022 16:22

minor fixups

0117e30

some mountains terrain updates

6a52148

some minor VILLAGE updates

0c671a2

cleanup of core terrain-graphics.cfg

20ecc5d

doofus-01 marked this pull request as ready for review April 11, 2022 02:10

doofus-01 merged commit 19fcc14 into wesnoth:master Apr 16, 2022

doofus-01 added a commit to doofus-01/wesnoth that referenced this pull request Apr 24, 2022

fixing some bugs introduced by wesnoth#6606

63b018d

doofus-01 mentioned this pull request Apr 24, 2022

1.17 - minor terrain updates #6626

Merged

doofus-01 added a commit that referenced this pull request Apr 24, 2022

1.17 - minor terrain updates (#6626)

4e00e30

* updates to desert terrain * ^Bsa snowy stone bridges * fix wooden bridge bug introduced by #6606

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

1.17 terrain-graphics macros clean-up #6606

1.17 terrain-graphics macros clean-up #6606

doofus-01 commented Apr 3, 2022 •

edited

cooljeanius commented Apr 3, 2022

doofus-01 commented Apr 3, 2022

Pentarctagon commented Apr 3, 2022

doofus-01 commented Apr 4, 2022

Pentarctagon commented Apr 4, 2022

doofus-01 commented Apr 4, 2022

doofus-01 commented Apr 9, 2022

CelticMinstrel commented Apr 9, 2022 •

edited

CelticMinstrel Apr 9, 2022

doofus-01 Apr 9, 2022

CelticMinstrel Apr 9, 2022

CelticMinstrel Apr 9, 2022

Pentarctagon commented Apr 9, 2022

CelticMinstrel commented Apr 9, 2022

doofus-01 commented Apr 9, 2022

Pentarctagon commented Apr 9, 2022

CelticMinstrel commented Apr 10, 2022

CelticMinstrel Apr 10, 2022

doofus-01 Apr 10, 2022

doofus-01 commented Apr 11, 2022

doofus-01 commented Apr 16, 2022

Pentarctagon commented Apr 16, 2022

1.17 terrain-graphics macros clean-up #6606

1.17 terrain-graphics macros clean-up #6606

Conversation

doofus-01 commented Apr 3, 2022 • edited

cooljeanius commented Apr 3, 2022

doofus-01 commented Apr 3, 2022

Pentarctagon commented Apr 3, 2022

doofus-01 commented Apr 4, 2022

Pentarctagon commented Apr 4, 2022

doofus-01 commented Apr 4, 2022

doofus-01 commented Apr 9, 2022

CelticMinstrel commented Apr 9, 2022 • edited

CelticMinstrel Apr 9, 2022

Choose a reason for hiding this comment

doofus-01 Apr 9, 2022

Choose a reason for hiding this comment

CelticMinstrel Apr 9, 2022

Choose a reason for hiding this comment

CelticMinstrel Apr 9, 2022

Choose a reason for hiding this comment

Pentarctagon commented Apr 9, 2022

CelticMinstrel commented Apr 9, 2022

doofus-01 commented Apr 9, 2022

Pentarctagon commented Apr 9, 2022

CelticMinstrel commented Apr 10, 2022

CelticMinstrel Apr 10, 2022

Choose a reason for hiding this comment

doofus-01 Apr 10, 2022

Choose a reason for hiding this comment

doofus-01 commented Apr 11, 2022

doofus-01 commented Apr 16, 2022

Pentarctagon commented Apr 16, 2022

doofus-01 commented Apr 3, 2022 •

edited

CelticMinstrel commented Apr 9, 2022 •

edited