search

project-ethea / Invasion_from_the_Unknown

Invasion from the Unknown, a Battle for Wesnoth add-on campaign.
Other
10 stars 4 forks source link

TRAIT_LOYAL nonsense #84

Open irydacea opened 3 years ago

irydacea commented 3 years ago

In 1.15.x, TRAIT_LOYAL takes a parameter specifying the crown overlay image path, which we need to use here. We can't use IS_HERO anymore because it results in double overlays, with the hero crown being rendered behind the loyal crown if the code placement looks like this:

{IS_HERO}
[modifications]
    {TRAIT_LOYAL}
[/modifications]

... instead of this:

[modifications]
    {TRAIT_LOYAL}
[/modifications]
{IS_HERO}

CC #71

newfrenchy83 commented 3 years ago

i put a fix to this issue.

newfrenchy83 commented 3 years ago

@shikadiqueen in case or you don't see that, i put 3 PR for resolve this issue.

irydacea commented 3 years ago

Yes, I am aware, but I'm not entirely happy about the fix.

newfrenchy83 commented 3 years ago

Yes, I am aware, but I'm not entirely happy about the fix.

What must i change, please?

newfrenchy83 commented 3 years ago

@shikadiqueen i know what is not that what you must wait but i think in long term, in 1.15.x same if IS_LOYAL and TRAIT_loyal add same overlays, it remain what it is also a double overlays but not visible. and using of local copy are in case or {IS_HERO/LOYAL macro willbe deleetd a day of the core, same if it is not actually.

irydacea commented 3 years ago

I'm just really unhappy about the amount of code noise this whole thing they did in mainline creates for me. I feel like I'd rather take a wholly different approach and eschew the mainline macros entirely along with any version-specific workarounds since the building blocks (i.e. [effect] apply_to=ellipse/overlay/loyal) were already in 1.14.0. At the same time though, I need to decide what to do about the two-trait (<trait1> + <trait2>) heroes vs. the single-trait (loyal + <trait>) ones, since they are wrapped up in this whole mess.

Furthermore, fixing this bug does also necessarily involve testing and possibly fixing any instances of unit overlays (or the loyal trait itself) being replaced or removed ­— namely, IftU S19 and S23B (Galas/Elynia), AtS E1S10 (Galas/Elynia), E2S11 (Elynia/Anya) and E3S7B (Elynia/Anya + Durvan).

I just haven't put much thought into this yet because as a 1.15.x-specific issue it's not a priority for me, and no matter how much Konrad2 nags me about it my primary target will remain 1.14.x until 1.16 RC 1.