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

[lua/sql] Sarcopsylla NM #5625

Merged
merged 2 commits into from
May 8, 2024
Merged

[lua/sql] Sarcopsylla NM #5625

merged 2 commits into from
May 8, 2024

Conversation

MowFord
Copy link
Contributor

@MowFord MowFord commented May 5, 2024

I affirm:

  • I understand that if I do not agree to the following points by completing the checkboxes my PR will be ignored.
  • I understand I should leave resolving conversations to the LandSandBoat team so that reviewers won't miss what was said.
  • I have read and understood the Contributing Guide and the Code of Conduct.
  • 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?

Implements retail accuracy for Sarcopsylla

Steps to test these changes

Goto the mob in grauberg, fight it and see that !getstats shows proper data, casts the right set of spells, and it doesn't instagib from JA/crit/ws

-----------------------------------
mixins = { require('scripts/mixins/families/chigoe') }
mixins = { require('scripts/mixins/families/chigoe_nm') }
-----------------------------------
local entity = {}

entity.onMobInitialize = function(mob)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a TODO: check immunites
bg wiki claims it has some

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We added this nm's immunities to the mob pool, I'll confirm when I rebase tomorrow

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, I forgot you can do it through that too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea group defines: 65559 = 65536 + 16 + 4 + 2 + 1 (sleep, grav, bind, silence, petrify)

Would you prefer we use the addImmunity binding in the mob init moving forward (or convert this one)? Definitely seems cleaner and easier to review

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dunnow if that binding works properly. But if it does, I would definatelly prefer to use that. It would at least make it clear whats going on

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree.

I would add to that one more thing related to this:

Right now we have

  • SLEEP
  • SLEEP_LIGHT
  • SLEEP_DARK

We dont need all 3, at all. SLEEP_LIGHT + SLEEP_DARK == SLEEP.

I would propose to also start normalizing using DARK and LIGHT and 0ing regular sleep, for a future removal/substitution. We are missing ADDLE, for instance, iirc.

I know the cases where one mob is trully immune to 1 without the other can be counted with one hand, but still, I dont think that justifies having 3 sleeps.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Xaver-DaRed

SLEEP_LIGHT and SLEEP_DARK were added for server custom bs purposes so they could separate them without polluting upstream while we upstream only ever used SLEEP, because at that time upstream did not belive any retail mob could be immune rather than highly resistant to just one of the 2

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember. Still, now that we know the cases exist, we should just embrace those 2 and say bye bye to the old one, couse, again, it aint needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to set immunities via mob init and confirmed it works:
image

Also Xaver requested I go ahead and split out sleep immunities now:
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still, now that we know the cases exist

murder it is then!

@claywar claywar merged commit fc5141b into LandSandBoat:base May 8, 2024
11 checks passed
hooksta4 pushed a commit to hooksta4/LSB_Hook_fork that referenced this pull request May 13, 2024
* [sql] Sarcopsylla NM

* [lua] Sarcopsylla NM
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.

None yet

5 participants