Skip to content

[GEO] Ironing out details for Geomantic Reservoir implementations #2076

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

Merged
merged 10 commits into from
Jul 11, 2022

Conversation

CatsEyeXI
Copy link

I affirm:

  • I have paid attention to this example and will edit again if need be to not break the formatting, or I will be ignored
  • I have read and understood the Contributing Guide
  • I've 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?

I realize that this will probably need to be revised in some way, but I wanted to try to get the ball rolling on these as they do not seem to be implemented fully. From what I can gather, we can probably recycle the same event/params used here on all of the other reservoirs as long as we have granular control over the final "unlock spell" text.

image

Steps to test these changes

!changejob GEO 5
!pos 378.875 -39.304 58.313
!addspell 769
Click on the GR.

Copy link
Contributor

@zach2good zach2good left a comment

Choose a reason for hiding this comment

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

+1 cookie for using a retail cap.

Remember that we do a lot of the things you're already trying to do in other zone files/quest files/mission files etc. There are a lot of examples of how to do things.

I would probably break all of this out into a single global like I did here:
https://github.com/LandSandBoat/server/blob/base/scripts/globals/geomagnetic_fount.lua

Then each file will look like:

entity.onTrigger = function(player, npc)
    if xi.geomanticReservoir.onTrigger(player, npc) then
        return
    end
end

-- maybe there's an onEventUpdate?

entity.onEventFinish = function(player, csid, option)
    if xi.geomanticReservoir.onEventFinish (player, csid, option) then
        return
    end
end

And then you'll have a new global:

-----------------------------------
-- SOA Geomantic Reservoirs
-----------------------------------
require("scripts/globals/keyitems")
require("scripts/globals/missions")
-----------------------------------

xi = xi or {}
xi.geomanticReservoir = xi.geomanticReservoir or {}

-- A table of each zone, and what spell they give

xi.geomagneticFount.onTrigger = function(player, npc)
    -- Get the zone text ID from the player object
    -- Do your checks
    -- Send your various messageSpecial()'s, or whatever they are
end

xi.geomagneticFount.onEventFinish= function(...)
    etc.
end

Good luck 👍

@zach2good
Copy link
Contributor

Here are the user-friendly dumps for June-2022: https://github.com/zach2good/UpdateExtractor/tree/versions/2022-june/out

@CatsEyeXI
Copy link
Author

xi = xi or {}
xi.geomanticReservoir = xi.geomanticReservoir or {}

Thank you for the detailed explanation, I'm slowly trying to chug through the recommendations. One thing I have noticed, is that some zones have multiple reservoirs, such as La Theine. How would you handle such a case? Check for the NPCID to determine which spell to unlock?

@TeoTwawki
Copy link
Contributor

TeoTwawki commented Jun 22, 2022

xi = xi or {}
xi.geomanticReservoir = xi.geomanticReservoir or {}

Thank you for the detailed explanation, I'm slowly trying to chug through the recommendations. One thing I have noticed, is that some zones have multiple reservoirs, such as La Theine. How would you handle such a case? Check for the NPCID to determine which spell to unlock?

ID's shift and would then require an update to the script every time the client version changes, better to modify their name in the sql table so they are separate scripts (can append a number to the name like Geomantic_Reservoir_2)

@CatsEyeXI
Copy link
Author

xi = xi or {}
xi.geomanticReservoir = xi.geomanticReservoir or {}

Thank you for the detailed explanation, I'm slowly trying to chug through the recommendations. One thing I have noticed, is that some zones have multiple reservoirs, such as La Theine. How would you handle such a case? Check for the NPCID to determine which spell to unlock?

ID's shift and would then require an update to the script every time the client version changes, better to modify their name in the sql table so they are separate scripts (can append a number to the name like Geomantic_Reservoir_2)

brilliant!

@CatsEyeXI
Copy link
Author

if this method is agreeable, i can try to move forward with the rest of these...

@CatsEyeXI
Copy link
Author

realized i am missing a check here to see if player has required Indi-spell. Ill update soon.

@CatsEyeXI
Copy link
Author

I think this is ready for another review, please let me know if anything else needs changed.

@CatsEyeXI
Copy link
Author

Hi, is there anything I can do to help move this along?

@zach2good zach2good added the squash Reminder to squash commits before/on merge (you can do this, or maintainers will do this for you) label Jul 6, 2022
@zach2good zach2good merged commit b3bb88a into LandSandBoat:base Jul 11, 2022
@CatsEyeXI CatsEyeXI deleted the geomantic_reservoirs branch July 11, 2022 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
squash Reminder to squash commits before/on merge (you can do this, or maintainers will do this for you)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants