Feat: Add Card:can_sell() API method#1254
Feat: Add Card:can_sell() API method#1254NeatsTopFoo wants to merge 6 commits intoSteamodded:mainfrom
Card:can_sell() API method#1254Conversation
Identical to other API method patches - But from what I can see, looks like it needs to be in better_calc.toml? As the area where the API method needs to be directly follows an eternal check and the next thing after that is a return. May need to be moved to center.toml
Ret check changed to a more strict nil check as it was ignoring falsy values
|
Looks good to me! The only comment is that |
Card:can_sell() API method
Card:can_sell() API methodCard:can_sell() API method
|
Ok, taking a second look I spot two issues: First the patch returns two values when can_sell_card only returns one. Second, it might be a bit confusing to make the function need to explicitly return a boolean instead of boolean|nil. |
Yes. Although this is just directly lifted from other API implementations, my thought process was that since this would operate based on the developer addition of a function to modded Joker implementations that operate in the same way, with similar expectations, that this could be potentially leveraged with further refinement in case there's some kind of feasible use case for a second return value. I personally can't think of a use case (I did initially think of something potentially involving Eternal, but developers can and should just hook If it does turn out that there isn't a feasible use case, I don't mind removing the second return value and just expecting it to return a boolean, for simplicity. |
|
In that case the second value would be added alongside whatever feature uses it. I don't see a reason for it to be here. |
Identical to other API method patches - But from what I can see, looks like it needs to be in better_calc.toml? As the area where the API method needs to be directly follows an eternal check and the next thing after that is a return.
May need to be moved to center.toml
Additional Info: