🔧 Refactor: Update refreshBoostListMessage call with new parameter#1976
🔧 Refactor: Update refreshBoostListMessage call with new parameter#1976
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the refreshBoostListMessage function to accept a new boolean parameter updateSignupMessage, which controls whether the signup reaction message should be updated. The changes improve code organization by consolidating the conditional update logic within the refresh function.
Key Changes:
- Added
updateSignupMessageboolean parameter torefreshBoostListMessagefunction - Updated all 15 call sites to pass
falsefor the new parameter, except in specific scenarios where the contract transitions to/from full capacity - Refactored
getContractReactionsComponentsto wrap menu options inside a signup state check, returning empty components during signup - Added defensive checks before appending button components to prevent appending empty arrays
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/boost/message_redraw.go |
Modified function signature to add updateSignupMessage parameter and conditionally call updateSignupReactionMessage; added defensive check for empty button components |
src/boost/boost.go |
Updated multiple call sites with new parameter; added logic in AddFarmerToContract to pass true when contract becomes full; added handling in removal to update signup message when transitioning from full capacity |
src/boost/stones_pages.go |
Updated call with false parameter |
src/boost/stones.go |
Updated call with false parameter |
src/boost/boost_speedrun.go |
Updated call with false parameter |
src/boost/boost_slashcmd.go |
Updated call with false parameter |
src/boost/boost_reactions.go |
Updated three calls with false parameter |
src/boost/boost_menu.go |
Updated three calls with false parameter |
src/boost/boost_change.go |
Updated five calls with false parameter |
src/boost/boost_button_reactions.go |
Refactored getContractReactionsComponents to only create menu during non-signup states; added redundant state check |
src/boost/contract.go |
Added defensive check for empty button components |
src/boost/artifacts.go |
Updated call with false parameter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if previousBoosters != len(contract.Boosters) && previousBoosters == contract.CoopSize { | ||
| updateSignupReactionMessage(s, contract, loc) | ||
| } |
There was a problem hiding this comment.
The condition previousBoosters == contract.CoopSize appears incorrect. This checks if the contract was full before adding the guest, but if the contract was full, the guest would be added to the waitlist instead (see line 735 in AddFarmerToContract). The condition should likely be len(contract.Boosters) == contract.CoopSize to check if the contract just became full. However, with the new refactoring, AddFarmerToContract already handles this at line 924 via refreshBoostListMessage, so this entire conditional block may now be redundant and could potentially cause duplicate updateSignupReactionMessage calls when the condition is fixed.
| if contract.AltIcons != nil && len(contract.AltIcons) > 0 { | ||
| } | ||
|
|
||
| if contract.State != ContractStateSignup { |
There was a problem hiding this comment.
This condition contract.State != ContractStateSignup is redundant as it's already inside a block that begins with the same condition at line 469. The nested check at line 488 will always evaluate to true and can be removed.
No description provided.