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

[WIP][Nyzul] Rewrite Nyzul Isle Investigation #4190

Draft
wants to merge 8 commits into
base: base
Choose a base branch
from

Conversation

Xaver-DaRed
Copy link
Contributor

@Xaver-DaRed Xaver-DaRed commented Jul 29, 2023

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.
  • [Obviously no] 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?

Originally intended to contain all the rewrite, it became obvious it was far too large and far too complex to be reviewed in a sane manner.

So now, this PR will act as a guide for the smaller PRs that will be opened containing small chunks of the rewrite at a time, to, hopefully, make it simpler to review and to code without having to rebase multiple times.

It will also act as the final PR of the rewrite, couse I already opened it and why clutter the PR history?

Steps to test these changes

Half of it it's already in, so... Do nyzul and see if it works, I guess, I dunnow. What's nyzul?

@UmeboshiXI
Copy link
Contributor

If you need help testing anything, I'm willing to help out. I was getting ready to tackle this myself for our server.

Some things of note with the current implementation and the client crash when entering the instance. The client crash can be avoided if you are in the menu when instance leader/initiator confirms entry.

@Xaver-DaRed
Copy link
Contributor Author

Once it's ready, feel free to cherry-pick and test it. The more ppl helping testing it, the better.

@zach2good
Copy link
Contributor

When this is eventually ready for review, I think you're going to have to write a little review guide so we know where to look, because this is massive :D

@Xaver-DaRed Xaver-DaRed force-pushed the Nyzul/Rewrite branch 6 times, most recently from bdbc8ed to e5e918e Compare August 22, 2023 22:14
- Remove pathos logic and tables from nyzul global.
- Create global functions for gear mobs engage and death.
- Cleanup mob scripts and family mixin.
- Adjust pets.lua pathos lookup in case it gets changed.
- Renamed table names.
- Addopted a "first mob, last mob" pattern across all tables.
- Changed logic to reflect this changes.
- Renamed how dynamic tables are named, so we can now they are crated and edited on the spot.
- Changed how rates are handled, to make them clearer.
- Rename "NII_pathos" to "pathos". We are inside a folder, we dont need the NII_ subfix anymore.
- Rename addFloorPathos to addPathos
- Move logic from instance file to "floor_generation"
- Move data from nyzul global AND points file to "floor_generation"
- Adjust spawn point data to simplify logic
- Delete points file entirely.
- Delete references to said file.
- Cleanup logic for lamp positioning
- Cleanup and comenting mostly.
- Left a couple of TODOs in there
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