From 4092cd6d56de40d55bbce9df9c9246bdf863a2b2 Mon Sep 17 00:00:00 2001 From: thepaperpilot Date: Tue, 13 Feb 2024 06:48:56 -0600 Subject: [PATCH 1/4] Add regression test for modifier.getFormula respecting enabled --- src/game/modifiers.tsx | 18 +++++++++++++----- tests/game/modifiers.test.ts | 14 ++++++++++++++ 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/game/modifiers.tsx b/src/game/modifiers.tsx index 1ee3905..d80b45a 100644 --- a/src/game/modifiers.tsx +++ b/src/game/modifiers.tsx @@ -276,8 +276,12 @@ export function createExponentialModifier( export function createSequentialModifier< T extends Modifier[], S = T extends WithRequired[] - ? WithRequired - : Omit, "invert"> + ? T extends WithRequired[] + ? WithRequired + : Omit, "invert"> + : T extends WithRequired[] + ? WithRequired + : Omit, "invert"> >(modifiersFunc: () => T): S { return createLazyProxy(() => { const modifiers = modifiersFunc(); @@ -296,10 +300,14 @@ export function createSequentialModifier< : undefined, getFormula: modifiers.every(m => m.getFormula != null) ? (gain: FormulaSource) => - modifiers + modifiers.reduce((acc, curr) => { + if (curr.enabled == null || curr.enabled === true) { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return curr.getFormula!(acc); + } // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - .reduce((acc, curr) => Formula.if(acc, curr.enabled ?? true, - acc => curr.getFormula!(acc), acc => acc), gain) + return Formula.if(acc, curr.enabled, acc => curr.getFormula!(acc)); + }, gain) : undefined, enabled: modifiers.some(m => m.enabled != null) ? computed(() => modifiers.filter(m => unref(m.enabled) !== false).length > 0) diff --git a/tests/game/modifiers.test.ts b/tests/game/modifiers.test.ts index d5e186d..fdf0f67 100644 --- a/tests/game/modifiers.test.ts +++ b/tests/game/modifiers.test.ts @@ -199,6 +199,20 @@ describe("Sequential Modifiers", () => { // So long as one is true or undefined, enable should be true expect(unref(modifier.enabled)).toBe(true); }); + test("respects enabled", () => { + const value = ref(10); + const enabled = ref(false); + const modifier = createSequentialModifier(() => [ + createMultiplicativeModifier(() => ({ multiplier: 5, enabled })) + ]); + expect(modifier.getFormula(Formula.variable(value)).evaluate()).compare_tolerance( + value.value + ); + enabled.value = true; + expect(modifier.getFormula(Formula.variable(value)).evaluate()).not.compare_tolerance( + value.value + ); + }); }); describe("applies smallerIsBetter correctly", () => { From 2e0e221010ba4d0d792acafec14383962e804824 Mon Sep 17 00:00:00 2001 From: thepaperpilot Date: Tue, 20 Feb 2024 08:32:03 -0600 Subject: [PATCH 2/4] Made modifier typing a lot less nasty --- src/game/modifiers.tsx | 55 +++++++++++++++--------------------- src/util/common.ts | 8 ++++++ tests/game/modifiers.test.ts | 12 ++++---- 3 files changed, 37 insertions(+), 38 deletions(-) diff --git a/src/game/modifiers.tsx b/src/game/modifiers.tsx index d80b45a..ada19dc 100644 --- a/src/game/modifiers.tsx +++ b/src/game/modifiers.tsx @@ -4,7 +4,7 @@ import { jsx } from "features/feature"; import settings from "game/settings"; import type { DecimalSource } from "util/bignum"; import Decimal, { formatSmall } from "util/bignum"; -import type { WithRequired } from "util/common"; +import type { OmitOptional, OptionalKeys, RequiredKeys, WithRequired } from "util/common"; import type { Computable, ProcessedComputable } from "util/computed"; import { convertComputable } from "util/computed"; import { createLazyProxy } from "util/proxies"; @@ -38,16 +38,11 @@ export interface Modifier { description?: ProcessedComputable; } -/** - * Utility type used to narrow down a modifier type that will have a description and/or enabled property based on optional parameters, T and S (respectively). - */ -export type ModifierFromOptionalParams = undefined extends T - ? undefined extends S - ? Omit, "description" | "enabled"> - : Omit, "description"> - : undefined extends S - ? Omit, "enabled"> - : WithRequired; +/** Utility type that represents the output of all modifiers that represent a single operation. */ +export type OperationModifier = WithRequired< + Modifier, + "invert" | "getFormula" | Extract, keyof Modifier> +>; /** An object that configures an additive modifier via {@link createAdditiveModifier}. */ export interface AdditiveModifierOptions { @@ -65,9 +60,9 @@ export interface AdditiveModifierOptions { * Create a modifier that adds some value to the input value. * @param optionsFunc Additive modifier options. */ -export function createAdditiveModifier( +export function createAdditiveModifier>( optionsFunc: OptionsFunc -): ModifierFromOptionalParams { +) { return createLazyProxy(feature => { const { addend, description, enabled, smallerIsBetter } = optionsFunc.call( feature, @@ -111,7 +106,7 @@ export function createAdditiveModifier( )) }; - }) as unknown as ModifierFromOptionalParams; + }) as S; } /** An object that configures an multiplicative modifier via {@link createMultiplicativeModifier}. */ @@ -130,9 +125,10 @@ export interface MultiplicativeModifierOptions { * Create a modifier that multiplies the input value by some value. * @param optionsFunc Multiplicative modifier options. */ -export function createMultiplicativeModifier( - optionsFunc: OptionsFunc -): ModifierFromOptionalParams { +export function createMultiplicativeModifier< + T extends MultiplicativeModifierOptions, + S = OperationModifier +>(optionsFunc: OptionsFunc) { return createLazyProxy(feature => { const { multiplier, description, enabled, smallerIsBetter } = optionsFunc.call( feature, @@ -175,7 +171,7 @@ export function createMultiplicativeModifier )) }; - }) as unknown as ModifierFromOptionalParams; + }) as S; } /** An object that configures an exponential modifier via {@link createExponentialModifier}. */ @@ -196,9 +192,10 @@ export interface ExponentialModifierOptions { * Create a modifier that raises the input value to the power of some value. * @param optionsFunc Exponential modifier options. */ -export function createExponentialModifier( - optionsFunc: OptionsFunc -): ModifierFromOptionalParams { +export function createExponentialModifier< + T extends ExponentialModifierOptions, + S = OperationModifier +>(optionsFunc: OptionsFunc) { return createLazyProxy(feature => { const { exponent, description, enabled, supportLowNumbers, smallerIsBetter } = optionsFunc.call(feature, feature); @@ -263,7 +260,7 @@ export function createExponentialModifier( )) }; - }) as unknown as ModifierFromOptionalParams; + }) as S; } /** @@ -274,15 +271,9 @@ export function createExponentialModifier( * @see {@link createModifierSection}. */ export function createSequentialModifier< - T extends Modifier[], - S = T extends WithRequired[] - ? T extends WithRequired[] - ? WithRequired - : Omit, "invert"> - : T extends WithRequired[] - ? WithRequired - : Omit, "invert"> ->(modifiersFunc: () => T): S { + T extends Modifier, + S = WithRequired, keyof Modifier>> +>(modifiersFunc: () => T[]) { return createLazyProxy(() => { const modifiers = modifiersFunc(); @@ -325,7 +316,7 @@ export function createSequentialModifier< )) : undefined }; - }) as unknown as S; + }) as S; } /** An object that configures a modifier section via {@link createModifierSection}. */ diff --git a/src/util/common.ts b/src/util/common.ts index dbbe233..00847e6 100644 --- a/src/util/common.ts +++ b/src/util/common.ts @@ -1,3 +1,11 @@ +export type RequiredKeys = { + [K in keyof T]-?: NonNullable extends Pick ? never : K; +}[keyof T]; +export type OptionalKeys = { + [K in keyof T]-?: NonNullable extends Pick ? K : never; +}[keyof T]; + +export type OmitOptional = Pick>; export type WithRequired = T & { [P in K]-?: T[P] }; export type ArrayElements> = T extends ReadonlyArray diff --git a/tests/game/modifiers.test.ts b/tests/game/modifiers.test.ts index fdf0f67..3b2812f 100644 --- a/tests/game/modifiers.test.ts +++ b/tests/game/modifiers.test.ts @@ -133,14 +133,14 @@ describe("Exponential Modifiers", () => testModifiers(createExponentialModifier, "exponent", Decimal.pow)); describe("Sequential Modifiers", () => { - function createModifier( + function createModifier>( value: Computable, - options: Partial = {} - ): WithRequired { + options?: T + ) { return createSequentialModifier(() => [ - createAdditiveModifier(() => ({ ...options, addend: value })), - createMultiplicativeModifier(() => ({ ...options, multiplier: value })), - createExponentialModifier(() => ({ ...options, exponent: value })) + createAdditiveModifier(() => ({ ...(options ?? {}), addend: value })), + createMultiplicativeModifier(() => ({ ...(options ?? {}), multiplier: value })), + createExponentialModifier(() => ({ ...(options ?? {}), exponent: value })) ]); } From 1e2b20a70ff09eafe4ec1555f4e8b8fb0c7835a2 Mon Sep 17 00:00:00 2001 From: thepaperpilot Date: Tue, 20 Feb 2024 15:10:59 +0000 Subject: [PATCH 3/4] PR feedback --- tests/game/modifiers.test.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/game/modifiers.test.ts b/tests/game/modifiers.test.ts index 3b2812f..dd019e1 100644 --- a/tests/game/modifiers.test.ts +++ b/tests/game/modifiers.test.ts @@ -205,13 +205,10 @@ describe("Sequential Modifiers", () => { const modifier = createSequentialModifier(() => [ createMultiplicativeModifier(() => ({ multiplier: 5, enabled })) ]); - expect(modifier.getFormula(Formula.variable(value)).evaluate()).compare_tolerance( - value.value - ); + const formula = modifier.getFormula(Formula.variable(value)); + expect(formula.evaluate()).compare_tolerance(value.value); enabled.value = true; - expect(modifier.getFormula(Formula.variable(value)).evaluate()).not.compare_tolerance( - value.value - ); + expect(formula.evaluate()).not.compare_tolerance(value.value); }); }); From a39e65852d196a122bc598121012edb5a6d2ca42 Mon Sep 17 00:00:00 2001 From: thepaperpilot Date: Tue, 20 Feb 2024 19:23:14 -0600 Subject: [PATCH 4/4] Remove unused imports --- src/game/modifiers.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/game/modifiers.tsx b/src/game/modifiers.tsx index ada19dc..55efccb 100644 --- a/src/game/modifiers.tsx +++ b/src/game/modifiers.tsx @@ -4,7 +4,7 @@ import { jsx } from "features/feature"; import settings from "game/settings"; import type { DecimalSource } from "util/bignum"; import Decimal, { formatSmall } from "util/bignum"; -import type { OmitOptional, OptionalKeys, RequiredKeys, WithRequired } from "util/common"; +import type { RequiredKeys, WithRequired } from "util/common"; import type { Computable, ProcessedComputable } from "util/computed"; import { convertComputable } from "util/computed"; import { createLazyProxy } from "util/proxies";