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

SUNITEVENT_EventFunc_Handler should take D2C_EventTypes as argument instead of D2C_UnitEventCallbackTypes #129

Merged
merged 5 commits into from
Nov 16, 2023

Conversation

ajtos
Copy link
Contributor

@ajtos ajtos commented Nov 16, 2023

For some reason enums from D2C_UnitEventCallbackTypes were used as arguments for SUNITEVENT_EventFunc_Handler(). Adjusted also other places where that function was called.

Only mystery is SUNITEVENT_EventFunc_Handler(pGame, 13, pAttacker, 0, 0); in MonsterMode.cpp called inside sub_6FC641D0(). Is that unused event?

EVENT_DOACTIVE is not used anywhere. Maybe this is actual bug in D2 itself, where SUNITEVENT_EventFunc_Handler(pGame, 13, pAttacker, 0, 0); is called instead of SUNITEVENT_EventFunc_Handler(pGame, EVENT_DOACTIVE, pAttacker, 0, 0);?

@Necrolis
Copy link
Member

Necrolis commented Nov 16, 2023

Only mystery is SUNITEVENT_EventFunc_Handler(pGame, 13, pAttacker, 0, 0); in MonsterMode.cpp called inside sub_6FC641D0(). Is that unused event?

This is used by the reanimate stat to know when the corpse is valid to reanimate, see D2Game.0x6FD05B60

EVENT_DOACTIVE is not used anywhere. Maybe this is actual bug in D2 itself, where SUNITEVENT_EventFunc_Handler(pGame, 13, pAttacker, 0, 0); is called instead of SUNITEVENT_EventFunc_Handler(pGame, EVENT_DOACTIVE, pAttacker, 0, 0);?

This doesn't make sense, why would you call EVENT_DOACTIVE when a monster has died. Its more likely EVENT_DOACTIVE is present in dead code thats been COMDAT folded out of 1.10 (might be present in other versions). AFAIK this was added/enabled in D2R.

@Lectem
Copy link
Member

Lectem commented Nov 16, 2023

I think historically there were confusions between D2EventTimerStrc (which uses the somewhat badly named D2C_UnitEventCallbackTypes) and D2UnitEventStrc which says uint8_t nTimerType; //0x00 D2C_UnitEventCallbackTypes but actually should be using D2C_EventTypes according to what I'm seeing (D2Game.0x6FD156A0 calls SUNITEVENT_AllocTimer and sets nTimerType to a value from D2ItemStatCostTxt::wItemEvent from D2SkillsTxt::wAuraEvent which are both indices into events.txt...

events.txt:

event	*desc
hitbymissile	hit by a missile
damagedinmelee	damaged in melee
damagedbymissile	damaged by missile
attackedinmelee	melee attack atttempt
doactive	do active state skill
domeleedamage	do damage in melee
domissiledamage	do missile damage
domeleeattack	do melee attack
domissileattack	do missile attack
kill	killed something
killed	killed by something
absorbdamage	dealt damage
levelup	gain a level

This confirms SUNITEVENT_EventFunc_Handler event is an id from events.Txt (thus D2C_EventTypes)...

Would you be ok with fixing the comment and name of the field in D2UnitEventStrc ? And also documenting value 13 with above information from necrolis.

@Necrolis
Copy link
Member

@Lectem TBH the whole of SUnitEvent.h has poor naming since nothing about it relates to timers (its all trigger based). Might be best to shift that renaming to its own pull request.

@ajtos
Copy link
Contributor Author

ajtos commented Nov 16, 2023

I documented missing event, matching naming used by events.txt in D2R:

event *description
hitbymissile Unit is hit by a missile
damagedinmelee Unit takes damage from a melee attack
damagedbymissile Unit takes damage from a missile
attackedinmelee Unit is attacked by a melee attack
doactive Unit used a skill
domeleedamage Unit dealt damage with a melee attack
domissiledamage Unit dealt damage with a missile
domeleeattack Unit used a melee attack
domissileattack Unit used a missile attack
kill Unit killed another Unit
killed Unit dies
absorbdamage Unit takes damage
levelup Unit gained a Level
death Monster dies

Left everything else alone as suggested by Necrolis.

@Lectem Lectem merged commit 4eb46dc into ThePhrozenKeep:master Nov 16, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants