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

Add reclaimer to UnitDestroyed #1611

Open
sprunk opened this issue Jul 20, 2024 · 2 comments · May be fixed by #1669
Open

Add reclaimer to UnitDestroyed #1611

sprunk opened this issue Jul 20, 2024 · 2 comments · May be fixed by #1669
Labels
good first issue Good for newcomers Lua API

Comments

@sprunk
Copy link
Collaborator

sprunk commented Jul 20, 2024

Bill of materials to be done before starting this ticket:


Reclaim can kill a unit:

spring/rts/Sim/Units/Unit.cpp

Lines 2054 to 2058 in 19d1a00

// reclaim finished?
if (killMe || buildProgress <= 0.0f || health <= 0.0f) {
health = 0.0f;
buildProgress = 0.0f;
KillUnit(nullptr, false, true);

But it's very hard to detect that this happened (when using the "drain health" method of reclaim) because the unit suddently just dies with no attacker in UnitDestroyed.

Pass the builder as the killer to CUnit::KillUnit. This needs to be done after the weaponDefID ticket (#1639) says this was via reclaim, since otherwise you can't distinguish it from the builder just murdering the unit (e.g. via collision damage).

Do this also for other death events that have a culprit (e.g. pass transport for the "died with my transport", pass factory for the "i got cancelled" etc).

Caveats:

  • check whether non-nil attackerID and negative weaponDefID has a precedent. I think unit-unit collisions already do that but verify and say so in the PR.
@sprunk sprunk added the Lua API label Jul 20, 2024
@sprunk sprunk added the good first issue Good for newcomers label Aug 1, 2024
@keelefi
Copy link
Contributor

keelefi commented Aug 8, 2024

I propose this ticket be split in two. The first ticket adding weapondDefID as an argument in UnitDestroyed(). After that ticket has been completed, this ticket can be implemented.

@sprunk
Copy link
Collaborator Author

sprunk commented Aug 8, 2024

Sure. Added a bunch of tickets for each step (#1638, #1639) and added a bill of materials here in #1611.

@keelefi keelefi linked a pull request Aug 31, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers Lua API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants