Suggestion for {HIGHLIGHT_IMAGE} macro

Brainstorm ideas of possible additions to the game. Read this before posting!

Moderators: Forum Moderators, Developers

Forum rules
Before posting a new idea, you must read the following:
Post Reply
User avatar
WhiteWolf
Forum Moderator
Posts: 637
Joined: September 22nd, 2009, 7:48 pm
Location: Hungary

Suggestion for {HIGHLIGHT_IMAGE} macro

Post by WhiteWolf »

Hello,

I have a small suggestion for the above mentioned macro. Currently, only background images can be protected with the BACKGROUND parameter, but background halos are lost no matter what.
I'd suggest to change the BACKGROUND paramater to a yes/no boolean that would set if the background items (both images and halos, any number) would be protected, or erased during the blinking. This is possible by giving a name to the item being blinked. Here is an example of a possible new macro definition, highlighted where something is changed:
Proposed new macro def:
Great advantage is that it's backwards compatible with all {HIGHLIGHT_IMAGE x y image ()} calls, and behaves as it used to. Of course, if someone has {HIGHLIGHT_IMAGE x y image (specified_background)}, that has to be changed to {HIGHLIGHT_IMAGE x y image yes}

If it's worth the effort to extend backwards compatibility, then the definition

Code: Select all

#define HIGHLIGHT_IMAGE X Y IMAGE BACKGROUND_IMAGE
#arg PROTECT
no
#endarg

....
#enddef
could be used, where the new argument is optional for future use, and old code will still work. It has the advantage that if you want to protect all items on the location, you can use PROTECT=yes, and if you want to protect only one specific item, and erase all others, then you can specify that as BACKGROUND_IMAGE.
Author of the Underness Series, consisting of 5 parts: The Desolation of Karlag, The Blind Sentinel, The Stone of the North, The Invasion Of The Western Cavalry, Fingerbone of Destiny
Standalone works: The Ravagers - now for 1.14, with new bugs!
User avatar
josteph
Developer
Posts: 741
Joined: August 19th, 2017, 6:58 pm

Re: Suggestion for {HIGHLIGHT_IMAGE} macro

Post by josteph »

This is a bit hard to follow. If I understand correctly, your suggestion is to add a new version of {HIGHLIGHT_IMAGE} that, instead of taking {BACKGROUND_VALUE} that specifies a particular image to preserve, takes a yes/no parameter that means "preserve all images on that hex"? That sounds like a reasonable addition. I'm not sure if it's practical to make {HIGHLIGHT_IMAGE} support all of yes/no/()/filename for the fourth argument, though. If it is, great. If not, we can always add a new macro for the new functionality.
User avatar
WhiteWolf
Forum Moderator
Posts: 637
Joined: September 22nd, 2009, 7:48 pm
Location: Hungary

Re: Suggestion for {HIGHLIGHT_IMAGE} macro

Post by WhiteWolf »

josteph wrote:
October 18th, 2019, 1:27 am
If I understand correctly, your suggestion is to add a new version of {HIGHLIGHT_IMAGE} that, instead of taking {BACKGROUND_VALUE} that specifies a particular image to preserve, takes a yes/no parameter that means "preserve all images on that hex"? That sounds like a reasonable addition.
Yes, that's exactly the proposition. Let's suppose you have an item with a halo animation, or multiple items on that same hex, and you want to highlight the gohere.png for the player there. The current macro will erase your background no matter what (because the current implementation can only protect items with "image", and only one item at that).
In the meantime I defined the suggested macro for my add-on, and what's in the original post doesn't work, I had to do it a bit differently:
New macro that works and does what was suggested:
So if this macro was mainline, that's be cool.
I think it could be merged with the existing macro, so that the last argument can take any of yes/no/()/filename. You could do:

Code: Select all

{VARIABLE check_on_this {LAST_ARGUMENT}}
And then filter on its value. Firstly, yes and no are easy to filter for, then you can identify that it's a filename with

Code: Select all

[variable]
    name=check_on_this
    contains="png"
[/variable]
And then anything else is regarded as (). Then in the macro, behave accordingly to this variable. So I think it can be done. However, indeed I'm not sure either which would be the better idea: to have a new macro, or to make the old one support the new utility.
Author of the Underness Series, consisting of 5 parts: The Desolation of Karlag, The Blind Sentinel, The Stone of the North, The Invasion Of The Western Cavalry, Fingerbone of Destiny
Standalone works: The Ravagers - now for 1.14, with new bugs!
User avatar
josteph
Developer
Posts: 741
Joined: August 19th, 2017, 6:58 pm

Re: Suggestion for {HIGHLIGHT_IMAGE} macro

Post by josteph »

I'm leaning toward having two separate macros. With one macro it'll be two easy to forget to handle all case when we change the macro. Could you please (fix the indentation and) open a pull request for discussing this further?
User avatar
WhiteWolf
Forum Moderator
Posts: 637
Joined: September 22nd, 2009, 7:48 pm
Location: Hungary

Re: Suggestion for {HIGHLIGHT_IMAGE} macro

Post by WhiteWolf »

josteph wrote:
October 21st, 2019, 4:29 am
I'm leaning toward having two separate macros. With one macro it'll be two easy to forget to handle all case when we change the macro. Could you please (fix the indentation and) open a pull request for discussing this further?
So sorry for the very late answer :( I was completely absent for the past week.
Sorry for the indentation, I usually use tab(4), and when I copy mainline code and insert my stuff in it, usually this happens with the [code] on the forums. I'll fix that and open an issue :)
Author of the Underness Series, consisting of 5 parts: The Desolation of Karlag, The Blind Sentinel, The Stone of the North, The Invasion Of The Western Cavalry, Fingerbone of Destiny
Standalone works: The Ravagers - now for 1.14, with new bugs!
User avatar
Celtic_Minstrel
Developer
Posts: 1666
Joined: August 3rd, 2012, 11:26 pm
Location: Canada
Contact:

Re: Suggestion for {HIGHLIGHT_IMAGE} macro

Post by Celtic_Minstrel »

I'd rather see the functionality added as a new WML tag rather than extending the macro. I've even implemented it already, so if someone wanted to copy that to mainline, I wouldn't mind.
Author of The Black Cross of Aleron campaign and Default++ era.
Maintainer of Steelhive.
Post Reply