Dugy / Legend_of_the_Invincibles

An add-on campaign for the Battle for Wesnoth game
GNU General Public License v3.0
38 stars 21 forks source link

harm_unit_loti #824

Open white-haired-uncle opened 2 weeks ago

white-haired-uncle commented 2 weeks ago

1.

https://github.com/Dugy/Legend_of_the_Invincibles/blob/9d5eb9b9020cd891187980e1d7fa1de2cdc5ab24/lua/main.lua#L82-L87

I suspect this is newer than [hul]:

https://wiki.wesnoth.org/LuaAPI/wml-utils#utils.scoped_var

2. https://github.com/Dugy/Legend_of_the_Invincibles/blob/9d5eb9b9020cd891187980e1d7fa1de2cdc5ab24/lua/main.lua#L133-L136

I suspect this may not work correctly (facing is probably ignored) due to

https://github.com/wesnoth/wesnoth/issues/9032

3.

Probably a reason for this, but I can't see it

https://github.com/Dugy/Legend_of_the_Invincibles/blob/9d5eb9b9020cd891187980e1d7fa1de2cdc5ab24/lua/main.lua#L272

Dugy commented 2 weeks ago
  1. The code for [harm_unit_loti] is one of the oldest pieces of lua code in this codebase, from times when wesnoth's lua API contained much less stuff. I copied it from wesnoth's [harm_unit] implementation and changed the way experience was given. It's very likely some things are done in very old ways and can be done better now.
  2. Yeah, it's probably wrong, but it doesn't manifest because the unit is always already facing the target when animate=yes is used.
  3. No idea. I think it's some kind of mistake.
white-haired-uncle commented 2 weeks ago

Yeah, I doubt anything here actually causes an issue, but there was a discussion on discord this am about whether [harm_unit] should call [attacker_hits],etc or if that should be left up to UMC which got me looking at hul and I saw a few things worth at least mentioning.