server icon indicating copy to clipboard operation
server copied to clipboard

[lua]Prevents Ix'aern (Dragoon) from being popped by despawn

Open Frankie-hz opened this issue 9 months ago • 6 comments

You can currently despawn all the mobs and pop Ix'aern

I affirm:

  • [x] I understand that if I do not agree to the following points by completing the checkboxes my PR will be ignored.
  • [x] I understand I should leave resolving conversations to the LandSandBoat team so that reviewers won't miss what was said.
  • [x] I have read and understood the Contributing Guide and the Code of Conduct.
  • [x] I have tested my code and the things my code has changed since the last commit in the PR and will test after any later commits.

What does this pull request do?

Moves the logic for spawning Ix'aern (Dragoon) from despawn function to death

This was making it possible to pull any of the PHs and zone/despawn them in order to pop the NM

Steps to test these changes

Look up the variable [SEA]IxAernDRG_PH and get the PH ID. !goto PH ID !despawnmob See that Ix'DRG does not spawn anymore from despawning the PH !spawnmob PH ID !hp 0 the PH watch Ix'DRG spawn

Frankie-hz avatar Apr 30 '24 03:04 Frankie-hz

I think you want that to be wrapped in the isKiller check so that it doesn't run per party-alliance member maybe? And probably a nil check for unclaimed kills. Just guessing.

Shouldn't be necessary the onKiller check is just the player hate. If the PH dies without a target the mob should still spawn since the PH was still "killed". I can put it in there if you want but I don't think its necessary unless I am misunderstanding how the death function works. I tested this both by claiming it and killing it with no target/idle and it didn't cause any errors.

its a weird one and this likely was done the way it was because with other many NM, you absolutely can pop them by just repeatedly despawning the PH on retail (when the PH is allowed to despawn - some won't). I suspect what retail is actually doing for lotto pops is looking for is the "slot" is free and the NMs window is open (with smaller window intervals within it, they tend to have an alignment to a cycle)

This is the first time I am hearing about being able to depop PHs to spawn a lotto NM though. I was always told/observed never being able to pop a lotto NM by despawn. I'll do some more digging on that though, that should be pretty easy to test but yeah this is not one of those.

Frankie-hz avatar Apr 30 '24 14:04 Frankie-hz

We have optParams.isKiller optParams.noKiller

I have to agree with Teo this logic should be wraped inside a check for both, so it runs once and no more. The onMobDeath logic runs once per person in the mob enmity list. Using both of them

if
    optParams.isKiller or
    optParams.noKiller
then

Ensures it only runs once. Its a performance optimization, even if it doesnt ave any visible side-effects.

Xaver-DaRed avatar May 05 '24 12:05 Xaver-DaRed

I also have to wonder if we REALLY need to use a server variable for this. Server variables are slow AF and from my experience, sometimes they fail to update properly, specially if the code is run multiple times.

Xaver-DaRed avatar May 05 '24 12:05 Xaver-DaRed

I also have to wonder if we REALLY need to use a server variable for this. Server variables are slow AF and from my experience, sometimes they fail to update properly, specially if the code is run multiple times.

Volatile server variable would probably be better, yeah. but outside the scope of this pr

WinterSolstice8 avatar May 08 '24 02:05 WinterSolstice8

Does this need to persist? Could we get away with a localvar instead?

claywar avatar May 11 '24 20:05 claywar

Does this need to persist? Could we get away with a localvar instead?

Could just use a zone local var, would have the same outcome since the server var is set on the zone init

Frankie-hz avatar May 11 '24 21:05 Frankie-hz

This is fine to go, it just needs rebasing so it'll pick up the new required CI job

zach2good avatar Oct 03 '24 19:10 zach2good