Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Level up removes loyal overlay from unit #4058

Open
DisherProject opened this issue Apr 29, 2019 · 12 comments

Comments

Projects
None yet
7 participants
@DisherProject
Copy link
Contributor

commented Apr 29, 2019

(Version 1.15.0 on Windows 10)

To reproduce, you can load this game save...

2p — Caves of the Basilisk-Auto-Save1.gz

... and then attack the Blood Bat with your Elvish Fighter. You'll see the loyal overlay is removed after the advancement.

@sevu

This comment has been minimized.

Copy link
Member

commented Apr 30, 2019

When a unit levels up, it looses changes which were made to it directly
The new data is determined by:

  • the new unit type
  • the [traits], [objects] and amlas the unit had (and still has) … in fact, the part which you can see in debug mode in the [modifications] block

A bit unclear is how the attributes like hp, moves etc. are treated… but many are overwritten on level up by the above.

@sevu sevu closed this Apr 30, 2019

@jostephd

This comment has been minimized.

Copy link
Member

commented May 6, 2019

I can reproduce this in SP without mods, just by letting a loyal unit advance. Looks like a regression, so reopening.

See also https://forums.wesnoth.org/viewtopic.php?f=21&t=39771&start=30#p634215

@jostephd jostephd reopened this May 6, 2019

@jostephd jostephd added this to the 1.15.0 milestone May 6, 2019

@jostephd

This comment has been minimized.

Copy link
Member

commented May 6, 2019

To be clear - I started EI S1, let the loyal unit advance, and it lost its silver ring overlay. That's a regression with respect to 1.14, regardless of what changes may have been done under the hood.

@gfgtdf

This comment has been minimized.

Copy link
Contributor

commented May 6, 2019

I think the underlying change was most likeley an intentional change to fix the overlay [effect] in particular for [remove_object]. So it's probably best to.fix this in the wml.

@GregoryLundberg

This comment has been minimized.

Copy link
Contributor

commented May 7, 2019

Sounds to me like a design oversight. There is no "permanent" option for "overlay" to indicate whether it should be removed when the unit advances, (or, if there is, the loyal attribute forgot to use it.)

@sevu

This comment has been minimized.

Copy link
Member

commented May 7, 2019

Do the [unit_overlay] and [remove_unit_overlay] tags treat things differently?

@gfgtdf

This comment has been minimized.

Copy link
Contributor

commented May 7, 2019

Do the [unit_overlay] and [remove_unit_overlay] tags treat things differently?

Yes unit overlays added via unit_overlay are supposed to be removed via [remove_object], remove_unit_overlay only exists for compatability.

newfrenchy83 added a commit to newfrenchy83/wesnoth that referenced this issue May 9, 2019

fix level up emove overlay from unit
this fix wesnoth#4058 in hope to not generate another bug
@spixi

This comment has been minimized.

Copy link
Contributor

commented May 13, 2019

Wouldn't it make more sense, when loyal units always get that overlay?
What about having a overlay_image= key in [trait]?

This would make the {IS_LOYAL} boilerplate unnecessary. (You maybe remember {MAGENTA_IS_THE_TEAM_COLOR}, which is now superfluous.) A little bit off-topic, but moving the description for specials and abilities to adding a description key to [special] and [ability] would also help to tet rid of the WEAPON_SPECIAL_* and ABILITY_* marcos.

@newfrenchy83

This comment has been minimized.

Copy link
Contributor

commented May 13, 2019

And how dping for hero overlay, must i crete a hero trait and repla trait_loyal by trait_hero for all hero in core, and impose rhat of addon develloper, my splution is not ideal but have advantage to replace overlays multiple by simgle overlay naturelly persistznt without using [modification s].

newfrenchy83 added a commit to newfrenchy83/wesnoth that referenced this issue May 14, 2019

replace overlays by overlay
i create a overlay different to overlays for put in image of crown and resolve wesnoth#4058
@spixi

This comment has been minimized.

Copy link
Contributor

commented May 16, 2019

I guess, this is the line responsible for the removement of the overlay:

overlays_.clear();

@newfrenchy83

This comment has been minimized.

Copy link
Contributor

commented May 16, 2019

i already try to use in 0dbb2c0 but this code is necessary for resolve accumulation of overlay after each advancement. i try to solve that in #4081

spixi added a commit to spixi/wesnoth that referenced this issue May 17, 2019

@spixi

This comment has been minimized.

Copy link
Contributor

commented May 17, 2019

gfgtdf added a commit to gfgtdf/wesnoth that referenced this issue May 20, 2019

move loyal overlays to the loyal trait
this in particular fixes wesnoth#4058 . Also people simply forgetting the
IS_LOYAL macro was a not-so-uncommon cause of bugs.

In some cases people want to give the LOYAL trait without giving the
loyal overlays for those cases i added a optional OVERLAY parameter to
the {TRAIT_LOYAL} macro, also heroes should now use {TRAIT_LOYAL_HERO}
instead which uses the hero overlay. The IS_LOYAL and IS_HERO macros are
no longer needed in combination with those traits.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.