server
server copied to clipboard
LandSandBoat
fixes "A Boy's Dream" blocking if a player has the odonto ra/ex
I affirm:
- [x] I have paid attention to this example and will edit again if need be to not break the formatting, or I will be ignored
- [x] I have read and understood the Contributing Guide
- [x] 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?
Fixes #2729 by checking for the appropriate CS value during the trade instead of checking if they don't have the odonto for the giant shell bug trade.
Steps to test these changes
Start "A Boy's Dream" quest for PLD AF. Get some giant shell bugs but don't trade one to Ailbeche yet. Go Get the odonto ra/ex as per usual for quest.
Before: If you did this before, Ailbeche would not progress the quest due to the lua checking that the player doesn't have the Odonto ra/ex needed for this quest.
After: Ailbeche will not accept the giant shell bug and properly progress the quest, talking to him will repeat the previous message he gives after a trade (In testing, he'd say "I wish could go fishing with my father..." again before repeating his last appropriate statement, so players may have to talk to him more than once if they need him to repeat it for some reason). He will not accept a repeat trade of the giant shell bug / odonto.
This same fix also prevents players accidently rolling back quest progression by trading the shell bug to Ailbeche after the odonto.
How I tested:
Did the previous AF quest before this.
Gave myself a giant shell bug and odonto via !additem
This was tested on a clean LAN LSB server
is that better?
sorry it pulled the name from my commit message.
normally, both the commit message AND the pr title should be like a summary of what you've actually done (commit messages are suggested to be like you are giving git orders cook fries before putting them in the basket) and then "fixes #number would be tacked on the next line of the commit message and in the description body of the pr for best results. We don't enforce a particular style, but we do ask for descriptive naming in both PR titles and in commit messages so we can look in the history of both local copies and github merges and see what it actually was at a glance.
Links that may help with writing meaningful messages, and a short sample from them:
https://www.freecodecamp.org/news/how-to-write-better-git-commit-messages/
To come up with thoughtful commits, consider the following:
- Why have I made these changes?
- What effect have my changes made?
- Why was the change needed?
- What are the changes in reference to?
Assume the reader does not understand what the commit is addressing. They may not have access to the story addressing the detailed background of the change.
https://initialcommit.com/blog/git-commit-messages-best-practices
As a general rule, your messages should start with a single line that’s no more than about 50 characters and that describes the changeset concisely, followed by a blank line, followed by a more detailed explanation.
And finally, this is from the author of git himself:
https://github.com/torvalds/subsurface-for-dirk/blob/a48494d2fbed58c751e9b7e8fbff88582f9b2d02/README#L88
Header line: explain the commit in one line (use the imperative)
Body of commit message is a few lines of text, explaining things in more detail, possibly giving some background about the issue being fixed, etc etc.
The body of the commit message can be several paragraphs, and please do proper word-wrap and keep columns shorter than about 74 characters or so. That way "git log" will show things nicely even when it's indented.
Make sure you explain your solution and why you're doing what you're doing, as opposed to describing what you're doing. Reviewers and your future self can read the patch, but might not understand why a particular solution was implemented.
@TeoTwawki ok I'll try to be more mindful of this.
@anyone
Is this ready to be merged yet? I'm waiting on this to update my stuff so I can move on to other things
You've changed two conditions here requiring a charvar that I don't see a counterpart to update, which means in order to review we'll need to go through the entire line to verify:
Also, there is a conflict that needs to be resolved, so a rebase will be necessary.
You've changed two conditions here requiring a charvar that I don't see a counterpart to update, which means in order to review we'll need to go through the entire line to verify:
Also, there is a conflict that needs to be resolved, so a rebase will be necessary.
I don't quite get the first part of this, I PRed this because of the comments in the relevant issue.
As for the conflict, this is due to how long it takes to get reviews when everyone is busy... I can drop the PR and redo it when we can decide what needs to be done, but I'd prefer live discussion with someone on Discord over waiting for reviews, as then I can just PR something that'll get merged instead of risking conflicts because its taking so long. Been told thats annoying though...
It isn't merge yet because I didn't finish all possible configurations before I went on holiday, but I've started a git guide with steps for how to rebase and squash:
https://github.com/LandSandBoat/lsb-wiki/blob/560d0eb067c31789f1c7c76629dfc57558eda8fa/Git-Guide.md
Seems like there's a button to work through the merge conflict online.
As with all tech and programming things, there are seas of good tutorials on YouTube
It isn't merge yet because I didn't finish all possible configurations before I went on holiday, but I've started a git guide with steps for how to rebase and squash:
https://github.com/LandSandBoat/lsb-wiki/blob/560d0eb067c31789f1c7c76629dfc57558eda8fa/Git-Guide.md
I thought it was the reviews holding it up cause it was still saying it needed review.