Skip to content

fix: don't assume first death has Spirit Pet#49

Open
lunaynx wants to merge 2 commits intoodtheking:mainfrom
lunaynx:fix/death-spirit-check
Open

fix: don't assume first death has Spirit Pet#49
lunaynx wants to merge 2 commits intoodtheking:mainfrom
lunaynx:fix/death-spirit-check

Conversation

@lunaynx
Copy link
Contributor

@lunaynx lunaynx commented Jan 7, 2026

Added a check to score calculation whether the player who died first has a Legendary Spirit Pet, rather than assuming they do.

@FlyModeZ
Copy link
Contributor

FlyModeZ commented Jan 8, 2026

I have a suggestion in map info

display a ☠ emoji in map info if firstDeathHasSpirit

@lunaynx
Copy link
Contributor Author

lunaynx commented Jan 8, 2026

Yeah, it should probably be indicated in some way, what is that symbol?

One thing to note is that the mod will potentially assume No Spirit for a couple seconds until the API returns the response if we didn't already have the player's profile cached (e.g. from Better Party Finder). I think this makes sense because you want to avoid sending a false "300 score" message.

So maybe Spirit should be the one to have a special indicator. But then again, that's more visual clutter for the expected state in competent parties (if they die in the first place), so I'm not sure.

@odtheking
Copy link
Owner

We are currently working on modifying our backend to contain only dungeon related infromation so we don't have to fetch the full profile
Ideally I would wait until that is done before merging this PR
In addition I think storing a death penalty value instead of a firstDeath & firstDeathHasSpirit inside DungeonListener can be a cleaner implementation where if penalty == 0 then check for spirit otherwise just add 2

@lunaynx
Copy link
Contributor Author

lunaynx commented Jan 9, 2026

We are currently working on modifying our backend to contain only dungeon related infromation so we don't have to fetch the full profile Ideally I would wait until that is done before merging this PR

Yeah, that's fine.

In addition I think storing a death penalty value instead of a firstDeath & firstDeathHasSpirit inside DungeonListener can be a cleaner implementation where if penalty == 0 then check for spirit otherwise just add 2

I don't think this would be really cleaner. Your proposed implementation risks the death penalty getting out of sync with the death count if there's a mistake in the code.

Also, since the profile fetch is done asynchronously and may not be immediately available, it may take time for the score to update. I would think it's better to be conservative and assume a lower score until the information is available instead of falsely triggering a 300 score alert at 298 or 299 score. But if you really want to go that route, I suppose we could always add 2 to the death penalty and then asynchronously remove 1 if it's confirmed the player has a Spirit Pet.

@odtheking
Copy link
Owner

Can use the death count for the if statement determining whether or not to fetch for spirit which removes the "firstDeath" variable

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