feat: secrets per run

[davishi25]

calculates and displays profile specific secrets/run on /skytilscata and party finder stats
this is a redo of my previous pr due to me messing up that branch

[gemini-code-assist]

Hello @davishi25, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

Hello! Gemini here, providing a summary of this pull request. This PR, titled "feat: secrets per run", aims to enhance the information displayed in the /skytilscata command and the Party Finder stats feature by adding a new metric: "Secrets Per Run". This metric is calculated specifically for the player's active profile and is derived from their total secrets found divided by their total completed dungeon runs (including both normal and master mode runs, excluding the overall 'total' count). The PR updates the relevant command and feature code to fetch the necessary data, perform the calculation, and display the result in the respective UI elements.

Highlights

• New Metric: Secrets Per Run: Introduces a new statistic, 'Secrets Per Run', to provide insight into a player's secret finding efficiency per dungeon completion.
• Display in Cata Command: Adds the calculated 'Secrets Per Run' metric to the output of the /skytilscata command.
• Display in Party Finder Stats: Includes the 'Secrets Per Run' metric in the hover text displayed for players in the Party Finder stats feature.
• Calculation Logic: Calculates total runs by summing tier completions for individual floors (excluding the 'total' entry) in both normal and master mode dungeons, then divides total secrets by this run count, rounding to two decimal places.

Changelog

Click here to see the changelog

• hypixel-api
  • Updates the hypixel-api submodule to a newer commit (07c4450... from ee53968...), likely incorporating necessary data fields or fixes for fetching player statistics.
• src/main/kotlin/gg/skytils/skytilsmod/commands/impl/CataCommand.kt
  • Imports NumberUtil.roundToPrecision.
  • Adds logic to calculate total dungeon runs from tier_completions data, excluding the 'total' entry.
  • Calculates secretsPerRun by dividing total secrets by the calculated runs and rounding.
  • Appends the calculated 'Secrets/Run' value to the miscellaneous section of the /skytilscata command output.
• src/main/kotlin/gg/skytils/skytilsmod/features/impl/dungeons/PartyFinderStats.kt
  • Imports NumberUtil.roundToPrecision.
  • Adds logic to calculate total dungeon runs from tier_completions data, excluding the 'total' entry.
  • Calculates secretsPerRun by dividing total secrets by the calculated runs and rounding.
  • Adds the calculated 'Secrets Per Run' value to the hover text for the miscellaneous section in the Party Finder stats display.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a useful feature for calculating and displaying secrets per run in both the /skytilscata command and Party Finder stats. The core idea is good, and the changes are relatively straightforward. However, there are a couple of key areas that need attention.

Summary of Findings

• Potential Division by Zero: The calculation for secretsPerRun in both CataCommand.kt and PartyFinderStats.kt does not explicitly handle the case where the number of runs is zero. This can lead to division by zero, resulting in Infinity (if secrets > 0) which roundToPrecision then converts to a large finite number, or NaN (if secrets = 0) which roundToPrecision converts to 0.0. Displaying a very large number for secrets/run when there are no runs is misleading.
• Code Duplication: The logic to calculate secretsPerRun (including fetching completions, summing runs, and the division) is duplicated in CataCommand.kt and PartyFinderStats.kt. This violates the DRY principle and makes future modifications more error-prone.

Merge Readiness

This pull request adds a valuable feature. However, due to the critical issue of potential division by zero (which can lead to incorrect data display) and the high-severity issue of code duplication, I recommend that these changes be addressed before merging. Centralizing the calculation logic will also make it easier to apply the fix for the division by zero in one place. I am unable to approve pull requests, so please ensure these points are reviewed and addressed by the maintainers.

[gemini-code-assist]

There's a potential for division by zero here if runs is equal to 0.0. If runs is 0.0 and profileData.dungeons?.secrets is greater than 0, the division (profileData.dungeons?.secrets ?: 0.0) / runs would result in Infinity. The roundToPrecision(2) method then converts Infinity to a very large finite number, which would be misleading when formatted and displayed to the user. If profileData.dungeons?.secrets is 0.0 and runs is 0.0, the result is NaN, which roundToPrecision(2) converts to 0.0 (which is acceptable in this specific sub-case).

How about explicitly checking if runs is greater than zero before performing the division to prevent this and ensure a more accurate representation (e.g., 0 secrets/run if there are no runs)?

Suggested change

	val runs = comps.sum() - (cataData.tier_completions["total"] ?: 0.0) +
	masterComps.sum() - (masterCataData?.tier_completions?.get("total") ?: 0.0)
	val secretsFound = profileData.dungeons?.secrets ?: 0.0
	val secretsPerRun = if (runs > 0.0) {
	(secretsFound / runs).roundToPrecision(2)
	} else {
	0.0 // Default to 0.0 secrets/run if there are no runs to avoid division by zero
	}

[gemini-code-assist]

Similar to the CataCommand.kt file, there's a potential for division by zero if runs is 0.0. This can lead to secretsPerRun being a misleadingly large number (if secrets > 0 and runs = 0, Infinity is rounded) or 0.0 (if secrets = 0 and runs = 0, NaN is rounded to 0.0).

It's important to handle the case where runs is zero to prevent incorrect data display. Consider checking runs > 0.0 before division, as suggested for CataCommand.kt.

                    val dungeonSecrets = profileData.dungeons?.secrets ?: 0.0
                    val secretsPerRun = if (runs > 0.0) {
                        (dungeonSecrets / runs).roundToPrecision(2)
                    } else {
                        0.0 // Default to 0.0 secrets/run if there are no runs
                    }

[gemini-code-assist]

This block of logic for calculating comps, masterComps, runs, and secretsPerRun appears to be similar to the logic in CataCommand.kt (lines 132-136).

To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, could we extract this calculation into a shared utility function? This function could potentially reside in a relevant utility class (e.g., DungeonUtils or a new PlayerStatsUtils) and take cataData, masterCataData, and profileData.dungeons (or the specific secrets count) as parameters, returning the calculated secretsPerRun.

This would also centralize the fix for the potential division by zero issue.