search

MokhaLeee / fe8u-cskillsys-kernel

Morden c-skillsystem for fireemblem8u
MIT License
9 stars 6 forks source link

Skill - Eclipse #127

Closed JesterWizard closed 3 months ago

JesterWizard commented 3 months ago

Leave the enemy at 1HP if an attack would otherwise kill them (Skill % activation).

MokhaLeee commented 3 months ago

Your calculation method will cause the enemy's HP to be reduced to 1 regardless of whether the enemy is killed or not.

JesterWizard commented 3 months ago

Good catch, I added a conditional now.

MokhaLeee commented 3 months ago

There is a very clear time point to handle on target kill in BattleGenerateHit . The calculation logic of this PR should be consistent with the vanilla.

JesterWizard commented 3 months ago

I think I'll need a hint on this one, it's not 'very clear' to me. No matter where I put it in BattleGenerateHit it either doesn't activate, or displays the skill animation for the wrong side. And it both cases it doesn't do jack shit. You'd think sticking it in the else part of if (defender->unit.curHP == 0) would do the trick but nope.

MokhaLeee commented 3 months ago

There has already a skill named "SID_Bane" played the seem effect as this PR

MokhaLeee commented 3 months ago

Oh I find that the skill name is not consistant to it's effect:

bane should directly leave unit to one hp: https://fireemblem.fandom.com/wiki/Bane Mercy should leave unit as 1 hp if killing: https://fireemblem.fandom.com/wiki/Mercy Eclipse should neutralize the enemy unit's defenses https://fireemblem.fandom.com/wiki/Eclipse_(Skill)

The skill name, skill icon and the skill effect are incompatible in this PR, and the effect has been achieved by skill Mercy and the icon has been used by SID_Luna and SID_LunaAttack

MokhaLeee commented 3 months ago

https://github.com/MokhaLeee/fe8u-cskillsys-kernel/commit/32ab0837b4a97a503f3c1bec0c5ff99569dd94cc

JesterWizard commented 3 months ago

Oh I find that the skill name is not consistant to it's effect:

bane should directly leave unit to one hp: https://fireemblem.fandom.com/wiki/Bane Mercy should leave unit as 1 hp if killing: https://fireemblem.fandom.com/wiki/Mercy Eclipse should neutralize the enemy unit's defenses https://fireemblem.fandom.com/wiki/Eclipse_(Skill)

The skill name, skill icon and the skill effect are incompatible in this PR, and the effect has been achieved by skill Mercy and the icon has been used by SID_Luna and SID_LunaAttack

No the Eclipse skill in the ASM skill system is meant to function in the reverse order of Bane. It should only reduce your opponent's HP to 1 if you wouldn't otherwise kill them.

Though if you don't care for it I'll close this PR. Because the Tellius version is straight up broken and I have no intention of adding that.

MokhaLeee commented 3 months ago

Sure

No the Eclipse skill in the ASM skill system is meant to function in the reverse order of Bane.

I think this is an obvious naming error on community project. In either version of the original game, Eclipse always has a similar mechanic to the Luna rather than Mercy

JesterWizard commented 3 months ago

Sure no worries. I'll change the name to something else when I get time.

JesterWizard commented 3 months ago

I'm closing this for now.