Implement Board and Feature Rewrites #88

thepaperpilot · 2024-12-11T20:52:05Z

thepaperpilot commented

2024-12-11 20:52:05 +00:00

The feature rewrite and board rewrite had to be merged together, and both had a lot of bug fixes done during the process of updating the demo project. So I merged them into this one PR, which now has a very long changelog.

Removed jsx() and JSXFunction. You can now use JSX.Element like any other Computable value
joinJSX now always requires a joiner. Just pass the array of elements or wrap them in <> and </> if there's no joiner
Removed coerceComponent, computeComponent, and computeOptionalComponent; just use the render function now
It's recommended to now do <MyComponent /> instead of <component :is="myComponent" />
All features no longer take the options as a type parameter, and all generic forms have been removed as a result
Fixed forceHideGoBack not being respected
Removed deepUnref as now things don't get unreffed before being passed into vue components by default
Moved MarkNode to new wrapper, and removed existing mark properties
Moved Tooltip to new wrapper, and made it take an options function instead of raw object
VueFeature component now wraps all vue features, and applies styling, classes, and visibility in the wrapping div. It also adds the Node component so features don't need to
mergeAdjacent now works with rows and cols inside each other (perhaps should've used scss to reduce the amount of css this took)
CoercableComponent renamed to Renderable since it should be used with render
Replaced isCoercableComponent with isJSXElement
Replaced Computable and ProcessedComputable with the vue built-ins MaybeRefOrGetter and MaybeRef
convertComputable renamed to processGetter
Also removed GetComputableTypeWithDefault and GetComputableType, which can similarly be replaced
dontMerge is now a property on rows and columns rather than an undocumented css class you'd have to include on every feature within the row or column
Fixed saves manager not being imported in addiction warning component
Created vueFeatureMixin for simplifying the vue specific parts of a feature. Passes the component's properties in explicitly and directly from the feature itself
All features should now return an object that includes props typed to omit the options object and satisfies the feature. This will ensure type correctness and pass-through custom properties. (see existing features for more thorough examples of changes)
Replaced decorators with mixins, which won't require casting. Bonus amount decorators converted into generic bonus amount mixin. Removed effect decorator
All render functions now return JSX.Element. The JSX variants (e.g. renderJSX) (except joinJSX) have been removed
Moved all features that use the clickable component into the clickable folder
Removed small property from clickable, since its a single css rule (min-height: unset) (you could add a small css class and pass small to any vue feature's classes property, though)
Upgrades now use the clickable component
Added ConversionType symbol
Removed setDefault, just use ??=
Added isType function that uses a type symbol to check
Renderables no longer get wrapped in computed refs, because JSX.Elements don't like that (desyncs with the DOM)
Relatedly, a lot of display functions got fairly simplified, removing unnecessary local components
Added MaybeGetter utility type for something that may be a getter function or a static value (but not a ref)
Made Achievement.vue use a Renderable for the display. The object of components can still be passed to createAchievement
Made Challenge.vue use a Renderable for the display. The object of components can still be passed to createChallenge
Fixed some issues introduced by the rewrite that broke particles systems
General cleanup
Removed Grid feature
Replaced board feature with a new system where arbitrary SVG and DOM elements can be placed on a pannable and zoomable canvas

Fixes #1, #2, #48, #45, #44, #15, #62.

Associated docs PR: profectus/profectus-docs#13

The feature rewrite and board rewrite had to be merged together, and both had a lot of bug fixes done during the process of updating the demo project. So I merged them into this one PR, which now has a very long changelog. - Removed `jsx()` and `JSXFunction`. You can now use `JSX.Element` like any other `Computable` value - `joinJSX` now always requires a joiner. Just pass the array of elements or wrap them in `<>` and `</>` if there's no joiner - Removed `coerceComponent`, `computeComponent`, and `computeOptionalComponent`; just use the `render` function now - It's recommended to now do `<MyComponent />` instead of `<component :is="myComponent" />` - All features no longer take the options as a type parameter, and all generic forms have been removed as a result - Fixed `forceHideGoBack` not being respected - Removed `deepUnref` as now things don't get unreffed before being passed into vue components by default - Moved MarkNode to new wrapper, and removed existing `mark` properties - Moved Tooltip to new wrapper, and made it take an options function instead of raw object - VueFeature component now wraps all vue features, and applies styling, classes, and visibility in the wrapping div. It also adds the Node component so features don't need to - `mergeAdjacent` now works with rows and cols inside each other (perhaps should've used scss to reduce the amount of css this took) - `CoercableComponent` renamed to `Renderable` since it should be used with `render` - Replaced `isCoercableComponent` with `isJSXElement` - Replaced `Computable` and `ProcessedComputable` with the vue built-ins `MaybeRefOrGetter` and `MaybeRef` - `convertComputable` renamed to `processGetter` - Also removed `GetComputableTypeWithDefault` and `GetComputableType`, which can similarly be replaced - `dontMerge` is now a property on rows and columns rather than an undocumented css class you'd have to include on every feature within the row or column - Fixed saves manager not being imported in addiction warning component - Created `vueFeatureMixin` for simplifying the vue specific parts of a feature. Passes the component's properties in explicitly and directly from the feature itself - All features should now return an object that includes props typed to omit the options object and satisfies the feature. This will ensure type correctness and pass-through custom properties. (see existing features for more thorough examples of changes) - Replaced decorators with mixins, which won't require casting. Bonus amount decorators converted into generic bonus amount mixin. Removed effect decorator - All `render` functions now return `JSX.Element`. The `JSX` variants (e.g. `renderJSX`) (except `joinJSX`) have been removed - Moved all features that use the clickable component into the clickable folder - Removed `small` property from clickable, since its a single css rule (`min-height: unset`) (you could add a small css class and pass small to any vue feature's classes property, though) - Upgrades now use the clickable component - Added ConversionType symbol - Removed setDefault, just use `??=` - Added isType function that uses a type symbol to check - Renderables no longer get wrapped in computed refs, because JSX.Elements don't like that (desyncs with the DOM) - Relatedly, a lot of display functions got fairly simplified, removing unnecessary local components - Added `MaybeGetter` utility type for something that may be a getter function or a static value (but not a ref) - Made Achievement.vue use a Renderable for the display. The object of components can still be passed to `createAchievement` - Made Challenge.vue use a Renderable for the display. The object of components can still be passed to `createChallenge` - Fixed some issues introduced by the rewrite that broke particles systems - General cleanup - Removed Grid feature - Replaced board feature with a new system where arbitrary SVG and DOM elements can be placed on a pannable and zoomable canvas Fixes #1, #2, #48, #45, #44, #15, #62. Associated docs PR: https://code.incremental.social/profectus/profectus-docs/pulls/13

thepaperpilot added 52 commits 2024-12-11 20:52:06 +00:00

WIP on rewriting board

Run Tests / test (pull_request) Failing after 57s

Details

424bde0cdd

Add support for rendering VueFeatures in boards

Run Tests / test (pull_request) Failing after 47s

Details

1acfde134b

Add cnodes

Run Tests / test (pull_request) Failing after 46s

Details

f0e831ee8f

Use z-index to avoid changing render order aca56f6af6

Perf optimization 3fd8375031

Fix upgrade purchasing on drag

Run Tests / test (pull_request) Failing after 48s

Details

17b878e3be

Document boards 68b1401c2a

Add some tests for boards ad421ac759

Move board to src/game 2cfa450d96

Move common board node CSS to class 0e7ca27e3b

Revert changes in /data. Check the board-example branch for them instead 1b267edc32

Add link to docs in setupDraggableNode docstring

Run Tests / test (pull_request) Successful in 2m2s

Details

1c7824b550

Revert "Add link to docs in setupDraggableNode docstring" 68e6e3f37a

This reverts commit 1c7824b550.

Update fontsource dependencies 90300ce848

Update vite dependencies 8a9e106157

Bump some minor dependencies ae45f9bc2f

Switch from CJS to ESM 673f7790c7

Removed dynamic imports 6ba25f9abd

WIP on rewriting board 1cbe97251c

Add support for rendering VueFeatures in boards c64ac82a25

Add cnodes a75c8d81f8

Use z-index to avoid changing render order 6a17bbc29c

Perf optimization 5b33a0fceb

Fix upgrade purchasing on drag 8745304631

Document boards c6035f9077

Add some tests for boards 46d0a9aa2e

Move board to src/game e0da9588d2

Move common board node CSS to class 99227a2cb2

Revert changes in /data. Check the board-example branch for them instead f9c59f7636

Add link to docs in setupDraggableNode docstring 643bfccada

Revert "Add link to docs in setupDraggableNode docstring" 1e13da1129

This reverts commit 1c7824b550.

Update vitest 707aacc383

Update pixi (while staying at v6 until particle-emitter supports v8) dc8d0ecc95

Update vue 2dab35f7cf

Update TS 80d7a743bc

Update eslint and prettier 88abd53faf

Fix merge c93418bfec

Update deps some more

Run Tests / test (pull_request) Failing after 47s

Details

bd165da264

Fix build issues

Run Tests / test (pull_request) Failing after 58s

Details

052a01d3f7

Update container version

Run Tests / test (pull_request) Failing after 1m21s

Details

3eeff40910

Add rollup for linux

Run Tests / test (pull_request) Failing after 13s

Details

a676829d66

Remove node install

Run Tests / test (pull_request) Failing after 5s

Details

5ce3e64f5d

Update node

Run Tests / test (pull_request) Failing after 1m20s

Details

ccd685cb9c

Remove processedPropType and convert all components to composition API

Run Tests / test (pull_request) Failing after 1m8s

Details

1e5411d279

Remove _props abstraction (fixes #2 )

Run Tests / test (pull_request) Failing after 1m7s

Details

6c8dd66677

Ran lint:fix 528e703c88

Fix last lint issue

Run Tests / test (pull_request) Successful in 1m10s

Details

4987916900

Merge branch 'feat/update-deps' into feat/board-rewrite

Run Tests / test (pull_request) Failing after 1m43s

Details

99511288c9

Feature rewrite 83d41428eb

- Removed `jsx()` and `JSXFunction`. You can now use `JSX.Element` like any other `Computable` value
- `joinJSX` now always requires a joiner. Just pass the array of elements or wrap them in `<>` and `</>` if there's no joiner
- Removed `coerceComponent`, `computeComponent`, and `computeOptionalComponent`; just use the `render` function now
- It's recommended to now do `<MyComponent />` instead of `<component :is="myComponent" />`
- All features no longer take the options as a type parameter, and all generic forms have been removed as a result
- Fixed `forceHideGoBack` not being respected
- Removed `deepUnref` as now things don't get unreffed before being passed into vue components by default
- Moved MarkNode to new wrapper, and removed existing `mark` properties
- Moved Tooltip to new wrapper, and made it take an options function instead of raw object
- VueFeature component now wraps all vue features, and applies styling, classes, and visibility in the wrapping div. It also adds the Node component so features don't need to
- `mergeAdjacent` now works with grids (perhaps should've used scss to reduce the amount of css this took)
- `CoercableComponent` renamed to `Renderable` since it should be used with `render`
- Replaced `isCoercableComponent` with `isJSXElement`
- Replaced `Computable` and `ProcessedComputable` with the vue built-ins `MaybeRefOrGetter` and `MaybeRef`
- `convertComputable` renamed to `processGetter`
- Also removed `GetComputableTypeWithDefault` and `GetComputableType`, which can similarly be replaced
- `dontMerge` is now a property on rows and columns rather than an undocumented css class you'd have to include on every feature within the row or column
- Fixed saves manager not being imported in addiction warning component
- Created `vueFeatureMixin` for simplifying the vue specific parts of a feature. Passes the component's properties in explicitly and directly from the feature itself
- All features should now return an object that includes props typed to omit the options object and satisfies the feature. This will ensure type correctness and pass-through custom properties. (see existing features for more thorough examples of changes)
- Replaced decorators with mixins, which won't require casting. Bonus amount decorators converted into generic bonus amount mixin. Removed effect decorator
- All `render` functions now return `JSX.Element`. The `JSX` variants (e.g. `renderJSX`) (except `joinJSX`) have been removed
- Moved all features that use the clickable component into the clickable folder
- Removed `small` property from clickable, since its a single css rule (`min-height: unset`) (you could add a small css class and pass small to any vue feature's classes property, though)
- Upgrades now use the clickable component
- Added ConversionType symbol
- Removed setDefault, just use `??=`
- Added isType function that uses a type symbol to check
- General cleanup

Merge remote-tracking branch 'template/feature/feat-and-board-rewrite' into feat/board-feature-rewrite 4ce1b60a3d

Complete the rewrite 68da6c352e

Renderables no longer get wrapped in computed refs, because JSX.Elements don't like that (desyncs with the DOM)
Relatedly, a lot of display functions got fairly simplified, removing unnecessary local components
Added `MaybeGetter` utility type for something that may be a getter function or a static value (but not a ref)
Made Achievement.vue use a Renderable for the display. The object of components can still be passed to `createAchievement`
Made Challenge.vue use a Renderable for the display. The object of components can still be passed to `createChallenge`
Fixed some issues introduced by the rewrite that broke particles systems

Remove Grid

Run Tests / test (pull_request) Failing after 1m41s

Details

c61bf64b15

thepaperpilot added 1 commit 2024-12-11 21:12:37 +00:00

Lint

Run Tests / test (pull_request) Successful in 1m14s

Details

3a69603031

thepaperpilot changed title from ~~feature/feat-and-board-rewrite~~ to Implement Board and Feature Rewrites

2024-12-11 21:13:04 +00:00

thepaperpilot added 1 commit 2024-12-12 13:47:09 +00:00

Cleanup

Run Tests / test (pull_request) Failing after 1m49s

Details

5718abc013

thepaperpilot added 1 commit 2024-12-16 01:39:57 +00:00

Change bonus mixin

Run Tests / test (pull_request) Failing after 1m31s

Details

5bdb5ceed1

thepaperpilot added 1 commit 2024-12-25 17:18:13 +00:00

Don't convert functions with parameters

Run Tests / test (pull_request) Failing after 1m14s

Details

0a5f63ff04

thepaperpilot added 1 commit 2024-12-25 17:26:42 +00:00

Lint

Run Tests / test (pull_request) Successful in 1m21s

Details

9e65adee95

escapee reviewed 2024-12-26 01:13:25 +00:00

escapee left a comment

Tons of changes here, so many files completely rewritten, insane amount of work done in here. Most of the changes were at least similar - reworking boards, removing JSX, bringing display computations out of the Vue components and into their typescript counterparts, and of course simplifying the feature structure to not need so much processing and type coercion. After cutting out all the "duplicate" changes, I'm only seeing a couple little issues, all outside the main bulk of this PR.

src/components/Layer.vue

					
				@ -66,0 +42,4 @@

				    nodes: Layer["nodes"];

				    forceHideGoBack: Layer["forceHideGoBack"];

				    index: number;

				}>();

escapee commented

2024-12-26 00:42:12 +00:00

Since we're just grabbing the types from the Layer here rather than writing in their types directly, is there a reason we don't just do defineProps<Layer & { index: number }>(); instead?

Same logic would apply to other similarly-defined Vue files - Achievement, Bar, Challenge, etc.

Since we're just grabbing the types from the Layer here rather than writing in their types directly, is there a reason we don't just do `defineProps<Layer & { index: number }>();` instead? Same logic would apply to other similarly-defined Vue files - Achievement, Bar, Challenge, etc.

thepaperpilot commented

2024-12-26 01:24:50 +00:00

Unfortunately, whenever I tried doing that, the props wouldn't get properly recognized and the build would fail with this error:

[plugin:vite:vue] [@vue/compiler-sfc] Unresolvable type reference or unsupported built-in utility type

I know there's limitations on the types you can use directly, but honestly I'd expect these to work - they're just interfaces and the only utility type they tend to use is MaybeRef, a vue builtin. As you can see, connecting the properties directly works fine so I don't think its an issue with the MaybeRefs.

Unfortunately, whenever I tried doing that, the props wouldn't get properly recognized and the build would fail with this error: ```[plugin:vite:vue] [@vue/compiler-sfc] Unresolvable type reference or unsupported built-in utility type``` I know there's limitations on the types you can use directly, but honestly I'd expect these to work - they're just interfaces and the only utility type they tend to use is MaybeRef, a vue builtin. As you can see, connecting the properties directly works fine so I don't think its an issue with the MaybeRefs.

escapee commented

2024-12-26 04:08:28 +00:00

Ah, yeah, I'm getting weird errors when I try to do the same - some files I can make that change, others refuse to function.

escapee marked this conversation as resolved

src/data/projEntry.tsx

					
				@ -35,3 +31,4 @@

				    // Note: Casting as generic tree to avoid recursive type definitions

				    const tree = createTree(() => ({

				        nodes: [[prestige.treeNode]],

escapee commented

2024-12-26 00:39:24 +00:00

Not sure if it's a result of this PR or not, but this causes the node's tooltip's pinned to be saved in two different locations.

Not sure if it's a result of this PR or not, but this causes the node's tooltip's `pinned` to be saved in two different locations.

thepaperpilot commented

2024-12-26 01:26:15 +00:00

Ah, actually I was missing a noPersist around the nodes array. I did that in the demo and mention it in the docs, but didn't do it in the base project itself 💀.

thepaperpilot marked this conversation as resolved

tests/game/__snapshots__/modifiers.test.ts.snap Outdated

					
				@ -13,36 +13,7 @@ exports[`Additive Modifiers > applies description correctly > with description 1

				      "anchor": null,

				      "appContext": null,

				      "children": [

				        {

escapee commented

2024-12-26 00:57:38 +00:00

I'm not sure this file should exist, seems like temporary internal test data?

thepaperpilot commented

2024-12-26 01:29:43 +00:00

It should. Several of the tests just check that display components match the existing snapshots, since it can't really test that they render the same. The docs clarify the snapshots belong in version control here: https://vitest.dev/guide/snapshot#use-snapshots

The snapshot artifact should be committed alongside code changes, and reviewed as part of your code review process. On subsequent test runs, Vitest will compare the rendered output with the previous snapshot. If they match, the test will pass. If they don't match, either the test runner found a bug in your code that should be fixed, or the implementation has changed and the snapshot needs to be updated.

It should. Several of the tests just check that `display` components match the existing snapshots, since it can't really test that they render the same. The docs clarify the snapshots belong in version control here: https://vitest.dev/guide/snapshot#use-snapshots > The snapshot artifact should be committed alongside code changes, and reviewed as part of your code review process. On subsequent test runs, Vitest will compare the rendered output with the previous snapshot. If they match, the test will pass. If they don't match, either the test runner found a bug in your code that should be fixed, or the implementation has changed and the snapshot needs to be updated.

escapee marked this conversation as resolved

thepaperpilot added 1 commit 2024-12-26 01:30:32 +00:00

Fix error about pinned tooltips

Run Tests / test (pull_request) Successful in 1m15s

Details

595a4170b2

escapee reviewed 2024-12-26 04:14:37 +00:00

escapee left a comment

Going through this quickly one more time, have noticed a couple more things that should be quick to fix - then this'll all be good to go!

src/components/modals/Info.vue Outdated

					
				@ -82,3 +80,1 @@

				const infoComponent = computed(() => {

				    return coerceComponent(jsx(() => (<>{infoComponents.map(render)}</>)));

				});

				const Info = () => infoComponents.map(f => render(f));

escapee commented

2024-12-26 04:13:05 +00:00

This field name is overlapping with the component name, so while it seems to build fine, VSCode is expecting the template to contain a changelog prop, and I don't see any way to guarantee Vue will handle it properly either.

This field name is overlapping with the component name, so while it seems to build fine, VSCode is expecting the template to contain a `changelog` prop, and I don't see any way to guarantee Vue will handle it properly either.

thepaperpilot marked this conversation as resolved

src/features/achievements/Achievement.vue Outdated

					
				@ -35,0 +19,4 @@

				import { isJSXElement, render } from "util/vue";

				import { Component, isRef, unref } from "vue";

				import { Achievement } from "./achievement";

				import { displayRequirements } from "game/requirements";