-
Notifications
You must be signed in to change notification settings - Fork 629
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
[feat] Adds A Chocobo Riding Game #6675
base: base
Are you sure you want to change the base?
Conversation
Sorry its a beefy PR 😓 Please let me know if I went about anything the wrong way, happy to tear the whole thing apart if I should have done this a different way. I put this in a draft because I still need more captures to fully verify the race times. I also need to verify the windurst woods gate glyph teleport location. Last thing todo still that I was hoping for some help with is to add a vanadiel day check to rotate between the different races in each starter city. I need to follow https://www.mithrapride.org/vana_time/chocoracetable.html and I wasn't quite sure how to accomplish this. |
scripts/globals/chocobo.lua
Outdated
duration = 1800 + (player:getMod(xi.mod.CHOCOBO_RIDING_TIME) * 60) | ||
end | ||
|
||
if chocoGame then -- Start A Chocobo Riding Game |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Numeric values are not falsey on 0, so this will always default to true
} | ||
|
||
-- Checks if the NPC can start a race and the player hasn't already participated this week | ||
xi.chocoboGame.raceCheck = function(player, npc, zone) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given player and npc, there's probably not a good reason to need to pass zone into this function, as it can be derived
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got that adjusted to get the zone from the player
end | ||
|
||
-- Get the event param to tell the player which city race they are offering | ||
xi.chocoboGame.renterOnTrigger = function(player, destination, eventSucceed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this and onEventFinish, I'd almost prefer they're named something different if non-conforming to standard functions (onTrigger usually implies player, npc as args, whereas onEventFinish is player, csid, option, npc).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renaming them to startRaceEvent and beginRace
end | ||
|
||
-- Race complete event if a record for this path exists | ||
if recordName then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As previously, might want to confirm that an empty string is interpreted as false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My testing showed this working correctly, the messaging from the NPC changed on my test depending on if a record existed or not
49af687
to
c8ae610
Compare
I think I would separate the teleport items logic into a separate PR, considering they are "unrelated" enough and quite easy to immediately approve. |
c8ae610
to
5dc2f56
Compare
5dc2f56
to
cbb7ea1
Compare
Got that split out into #6676 |
I affirm:
What does this pull request do?
Captures are being tracked here
Steps to test these changes