Skip to content

Add High roller gamble (TMB Gambles)#6732

Open
TastyPumPum wants to merge 9 commits intooldschoolgg:masterfrom
TastyPumPum:high-roller-gamble
Open

Add High roller gamble (TMB Gambles)#6732
TastyPumPum wants to merge 9 commits intooldschoolgg:masterfrom
TastyPumPum:high-roller-gamble

Conversation

@TastyPumPum
Copy link
Contributor

@TastyPumPum TastyPumPum commented Jan 23, 2026

High Roller Pot lets players gamble gp stakes. Hosts set a stake and payout mode (winner-takes-all or top-3 split) and either invite specific players to confirm within 30 seconds or open the lobby for anyone to join. When the pot starts, all participants (including the host) are charged immediately, each rolls a random tradeable item, and the highest-value roll wins the pot (or top three split the payout).

  • I have tested all my changes thoroughly.

Important

Add High Roller Pot gambling feature with new command, image rendering, and tests.

  • Feature:
    • Add High Roller Pot gambling feature with high_roller command in gamble.ts.
    • Supports two payout modes: winner-takes-all and top-3 split.
    • Allows inviting specific players or opening lobby for all.
  • Image Rendering:
    • New drawHighRollerImage() in highRollerImage.ts to generate result images.
    • Handles up to 20 visible rolls, with overflow indication.
  • Command Logic:
    • Implement highRollerCommand() in highRollerCommand.ts for handling game logic.
    • Includes participant validation, roll generation, and payout distribution.
  • Tests:
    • Add tests for drawHighRollerImage() in highRollerImage.test.ts.
    • Add tests for payout calculation and roll generation in highRollerCommand.test.ts.

This description was created by Ellipsis for c51b267. You can customize this summary. It will automatically update as commits are pushed.

@TastyPumPum
Copy link
Contributor Author

@ellipsis-dev review this pr

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed everything up to c51b267 in 2 minutes and 53 seconds. Click for details.
  • Reviewed 869 lines of code in 6 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/mahoji/lib/abstracted_commands/highRollerCommand.ts:69
  • Draft comment:
    generateUniqueRolls re-rolls duplicate values using a safety counter. Consider potential performance issues if duplicates are frequent. Optimizing the uniqueness strategy may be beneficial.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 15% vs. threshold = 50% The comment is speculative in nature - it says "consider potential performance issues" and "may be beneficial" rather than identifying a concrete problem. The rules explicitly state "Do NOT make speculative comments, such as 'If X, then Y is an issue'. Only comment if it is definitely an issue." The function has a safety counter that throws an error after 1000 attempts, which is a reasonable safeguard. Without evidence that this is actually causing performance problems in practice, this is just a theoretical concern. The comment doesn't provide specific actionable guidance on what to change or how to optimize it. The function does have a safety mechanism in place (throws after 1000 attempts), so it's not completely unprotected. However, if duplicates are very frequent (e.g., with a small pool of items with the same price), this could theoretically be a real issue. The comment might be highlighting a legitimate concern about the algorithm's efficiency. While the concern could be theoretically valid, the comment is still speculative ("consider potential performance issues", "may be beneficial") without demonstrating that this is actually a problem. The rules are clear that speculative comments should be removed. If there was evidence of actual performance issues or a specific scenario where this fails, that would be different. The safety counter of 1000 attempts is reasonable for the max 50 participants. This comment should be deleted because it's speculative and doesn't identify a definite issue. It uses language like "consider" and "may be beneficial" which indicates it's not pointing to a concrete problem requiring a code change. The function has appropriate safeguards in place.
2. src/mahoji/lib/abstracted_commands/highRollerCommand.ts:388
  • Draft comment:
    In payoutWinners, economy transactions are performed sequentially. Consider wrapping these updates in a database transaction to ensure atomicity and avoid inconsistent state on failure.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment is suggesting a code quality improvement around atomicity. However, there are several issues with this comment: 1. It's speculative - it says "consider" which is a soft suggestion 2. The actual balance changes happen via addItemsToBank which may already have transaction handling 3. The economyTransaction records appear to be audit logs, not the actual source of truth for balances 4. The comment doesn't provide strong evidence that there's an actual problem 5. Without seeing the implementation of addItemsToBank and understanding the data model, it's hard to know if this is a real issue 6. The GP was already removed from participants before this function, so there's already potential for inconsistency that this wouldn't fix I might be missing that this is a legitimate concern about data consistency in a financial system where atomicity is critical. The comment could be pointing out a real bug where if the process fails partway through the payout loop, some winners get paid and others don't, creating an inconsistent state. While atomicity is important in financial systems, this comment is too speculative and doesn't demonstrate strong evidence of a problem. The comment uses "consider" which makes it a suggestion rather than identifying a definite issue. Additionally, without seeing the full context of how addItemsToBank works and whether it has its own transaction handling, this comment requires too much external context to validate. The rules state I should only keep comments with STRONG EVIDENCE that they are correct. This comment should be deleted. It's a speculative suggestion ("consider") about adding database transactions without demonstrating that there's actually a problem. Understanding whether this is needed requires knowledge of the addItemsToBank implementation and the broader transaction handling strategy, which is beyond the scope of this file review.
3. src/mahoji/lib/abstracted_commands/highRollerCommand.ts:579
  • Draft comment:
    The final response is sent via safeEdit() and then returned with interaction.returnStringOrFile(). Ensure that this double messaging pattern is intended and doesn't result in duplicate messages.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 40% <= threshold 50% The comment is asking the author to ensure that a double messaging pattern is intended and doesn't result in duplicate messages. This is a request for confirmation of intention, which violates the rules. However, it does point out a potential issue with the code logic, which could be useful for the author to consider. The comment could be rephrased to suggest reviewing the logic for potential duplication without asking for confirmation.
4. src/mahoji/commands/gamble.ts:218
  • Draft comment:
    High Roller Pot subcommand integration looks correct. Ensure that all validations (e.g. stake and invites) are properly handled in highRollerCommand.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to ensure that validations are properly handled, which falls under the rule of not asking the author to ensure behavior is intended or tested. It doesn't provide a specific suggestion or point out a specific issue.

Workflow ID: wflow_qeYt4q7vEMkI1fq8

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

1 participant