Forecast view #252

HaysCubing · 2025-01-17T20:14:52Z

"Forecast View" calculates projected averages for all competitors and then ranks the results based on projected average.

Projected average is shown for incomplete results.

HaysCubing · 2025-01-17T20:37:31Z

client/src/components/Round/Round.jsx

@@ -72,6 +76,12 @@ const ROUND_UPDATED_SUBSCRIPTION = gql`
  ${ROUND_RESULT_FRAGMENT}
 `;

+// Events sorted by best don't need forecast view.
+// Fewest moves is currently unsupported
+export function forecastViewDisabled(format, eventId) {


Is there a cleaner way to set up "forecastViewDisabled"? I want it to be disabled (icon and functionality) for "Best of" events and FMC. It looks like I need to pass the const "forecastView" since that's what's initialized with useState() and causes the page to reload. However calling forecastViewDisabled() within every child element generator seems inefficient.

Cleaned this up a bit. No longer passing to child functions. If you navigate from a page with it enabled (ex. 7x7) to a page disabled (BLD), and forecast view is enabled, then line 125 will disable it and do a page reload. So loads the page twice essentially - not sure if this is a big deal or not

HaysCubing · 2025-01-17T20:37:59Z

client/src/components/Round/RoundToolbar.jsx

@@ -23,6 +25,16 @@ function RoundToolbar({ round, competitionId }) {
      <Grid item style={{ flexGrow: 1 }} />
      {mdScreen && (
        <Grid item>
+          {!forecastViewDisabled(round.format, round.competitionEvent.event.id) &&


It's an icon for right now. Should it be a checkbox?

Made it a checkbox. More clear this way. Right now I have it so the entire thing doesn't appear at all if it's not supported. I could also just have it disabled for non supported events. Is that better?

HaysCubing · 2025-01-17T20:38:36Z

client/vite.config.js

@@ -15,5 +15,7 @@ export default defineConfig({
    proxy: {
      "/auth": "http://127.0.0.1:4000",
    },
+    watch: {usePolling: true},


Remove this before submitting

jonatanklosko · 2025-01-19T12:41:31Z

client/src/components/Round/Round.jsx

+  if (forecastView && forecastViewDisabled(round)) {
+    setForecastView(false);
+  }


I believe set* functions should not be called in the function body directly, but rather in response to some events. Here it may actually work because with the if makes sure it doesn't get called over and over, but I would do this instead:

Suggested change

if (forecastView && forecastViewDisabled(round)) {

setForecastView(false);

}

useEffect(() => {

// Reset to default on round change

setForecastView(false);

}, [roundId]);

jonatanklosko · 2025-01-19T12:43:33Z

client/src/components/Round/Round.jsx

+export function forecastViewDisabled(round) {
+  return round.format.sortBy === "best" || round.competitionEvent.event.id === "333fm";
+}


Is there a reason to exclude FMC? We only need to * 100 the values when computing average, as in the existing average function.

jonatanklosko · 2025-01-19T12:44:50Z

client/src/components/Round/RoundToolbar.jsx

+                  onChange={(event) => setForecastView(event.target.checked)}
+                />
+              }
+              label="Forecast View"


I think this takes too much space, but don't worry about it. I will experiment with a toggle icon before shipping.

jonatanklosko · 2025-01-19T13:02:34Z

client/src/lib/attempt-result.js

+      currentResult.ranking = i + 1;
+    }
+    // Using simple advancing logic.
+    // TODO: Implement questionable/clinched logic


We can actually leave advancingQuestionable untouched and it should be accurate. If advancingQuestionable is true, it means the result is advancing even if all remaining results are best possible, so it is still valid in the forecast view. If advancingQuestionable is false, then it should not become true based on projected average.

It may be worth leaving a comment that this is the case actually.

jonatanklosko · 2025-01-19T13:10:34Z

client/src/lib/attempt-result.js

+/**
+ * return the number of competitors that should be marked as advancing.
+ * When no competitors advance, return 3 for the podium placements.
+ * Might not be accurate for percentage based advancement but that is acceptable
+ */
+function getAdvancingCount(results, advancementCondition) {


Correct me if I'm wrong, but the point of forecast view is for final rounds, because it's usually those that have results entered partially. So what I would do is enable forecast view only for final rounds, and then for advancing we just mark top 3 ranking.

If a use case appears in the future to support other rounds, I think we should support all advancement conditions correctly.

jonatanklosko · 2025-01-20T06:33:45Z

client/vite.config.js

@@ -15,5 +15,7 @@ export default defineConfig({
    proxy: {
      "/auth": "http://127.0.0.1:4000",
    },
+    watch: {usePolling: true},


Suggested change

watch: {usePolling: true},

// Uncomment when running using Docker on Windows.

// See https://github.com/vitejs/vite/issues/1153#issuecomment-785467271

// watch: { usePolling: true },

jonatanklosko · 2025-01-20T07:33:51Z

client/src/components/Round/Round.jsx

@@ -72,6 +76,12 @@ const ROUND_UPDATED_SUBSCRIPTION = gql`
  ${ROUND_RESULT_FRAGMENT}
 `;

+// Events sorted by best don't need forecast view.
+// Fewest moves is currently unsupported
+export function forecastViewDisabled(round) {


Nit: let's move this to result.js also to keep the logic together, and let's revert as forecastViewEnabled (so that we don't need double negation as in !forecastViewDisabled(round)).

jonatanklosko · 2025-01-20T07:35:39Z

client/src/lib/attempt-result.js

+      forFirst: SKIPPED_VALUE,
+      forThird: SKIPPED_VALUE,


Let's remove for now, unless you are working on adding as part of the PR :)

jonatanklosko · 2025-01-20T07:35:54Z

client/src/lib/attempt-result.js

+ * forFirst: time needed to overtake first place
+ * forThird: time needed to overtake third place
+ */
+export function getExpandedResults(results, format, forecastView, advancementCondition) {


Let's move this to result.js.

Basically: attempt-result.js for various calculations on attempt result (numeric values), and result.js for dealing with result objects.

I would also rename this to resultsForView.

jonatanklosko · 2025-01-20T07:47:48Z

client/src/lib/attempt-result.js

+  const roundIncomplete = results.some((result) => isSkipped(result.average));
+
+  // Sort based on projection with tiebreakers on single
+  expandedResults = expandedResults.sort((a, b) => compareProjectedResults(a, b));


We can simplify a bit, first define this for attempt result:

function toMonotonic(attemptResult) { if (isComplete(attemptResult)) { return attemptResult; } else { return Infinity; } }

and then here we can do:

expandedResults = orderBy(expandedResults, [ (result) => toMonotonic(result.projectedAverage), (result) => toMonotonic(result.best), ]);

Forceast view

c54dfe2

HaysCubing commented Jan 17, 2025

View reviewed changes

HaysCubing added 2 commits January 17, 2025 12:51

Improve forecastViewDisabled function

fc47438

Checkbox

b12f056

jonatanklosko reviewed Jan 20, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Forecast view #252

Forecast view #252

HaysCubing commented Jan 17, 2025

HaysCubing Jan 17, 2025

HaysCubing Jan 17, 2025

HaysCubing Jan 17, 2025

HaysCubing Jan 17, 2025

HaysCubing Jan 17, 2025

jonatanklosko Jan 19, 2025

jonatanklosko Jan 19, 2025

jonatanklosko Jan 19, 2025

jonatanklosko Jan 19, 2025 •

edited

Loading

jonatanklosko Jan 19, 2025

jonatanklosko Jan 20, 2025

jonatanklosko Jan 20, 2025

jonatanklosko Jan 20, 2025

jonatanklosko Jan 20, 2025

jonatanklosko Jan 20, 2025

-    watch: {usePolling: true},
+    // Uncomment when running using Docker on Windows.
+    // See https://github.com/vitejs/vite/issues/1153#issuecomment-785467271
+    // watch: { usePolling: true },

Forecast view #252

Are you sure you want to change the base?

Forecast view #252

Conversation

HaysCubing commented Jan 17, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonatanklosko Jan 19, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonatanklosko Jan 19, 2025 •

edited

Loading