Deep ecosystem integration #11

ethangreen-dev · 2025-01-13T06:52:20Z

No description provided.

ebkr

Would also be good to update the adding a game document if this affects how we currently add games.

schemaSchema.json

src/model/game/StorePlatform.ts

src/model/schema/ThunderstoreSchema.ts

ebkr · 2025-01-22T11:00:49Z

schema.json

Is there a reason we have everything included?

Things like categories aren't necessary and don't need to be in here imo. It's a prime target for becoming out of sync and I'd imagine won't actually be used by us anyway.

For simplicity mostly. Otherwise we would need to have an additional build step with prunes all unnecessary fields which doesn't seem to offer much for the increase in complexity. This will be mostly ameliorated when we implement runtime schema updates.

anttimaki

I liked the parts where there were many consecutive red lines.

anttimaki · 2025-01-22T10:59:07Z

schemaSchema.json

@@ -0,0 +1,384 @@
+{
+    "$defs": {


I'm not entirely sure the defs/refs here make things easier to read, and they don't seem to be defined for reusability. Or is it a technical limitation of ajv? Just wondering if this would be easier to follow without the refs 🤷🏻

I'll look into customizing how pydantic exports the json schema. There's probably a way to prune these unnecessary fields.

anttimaki · 2025-01-22T10:59:22Z

schemaSchema.json

+            "title": "EntryR2",
+            "type": "object"
+        },
+        "EntryThunderstore": {


Why isn't this just called a "Community"?

These are the names of the corresponding pydantic dataclasses from the ecosystem schema repo. They shouldn't have any hold over how we define the types within r2mm, but I'm happy to change them if I'm incorrect.

anttimaki · 2025-01-22T10:59:26Z

schemaSchema.json

+            "title": "ModloaderPackage",
+            "type": "object"
+        },
+        "SchemaEntry": {


Why isn't this called just "Game" or something?

anttimaki · 2025-01-22T10:59:49Z

src/model/game/GameManager.ts

    }

    static get gameList(): Game[] {
-        return [...this._gameList];
+        return EcosystemSchema.r2mm_games.map((game) => {
+            const r2mm = game.r2modman!;


The ! doesn't seem to be necessary here?

As I'll mention later it's because we can't (or I don't know how) to tell the compiler "ok, we've checked and made sure that this field on the module is AOK and cannot be undefined".

At least Cursor can infer it directly from the current types, i.e. removing ! doesn't seem to affect inferred type.

anttimaki · 2025-01-22T10:59:52Z

src/model/game/GameManager.ts

+
+            // Map distribution values onto the r2mm StorePlatformMetadata type.
+            // The `identifier` property may not be present, so we check it beforehand to avoid JSON import lint errors.
+            const distributions = game.distributions.map((x: any) => new StorePlatformMetadata(


Why use any here when TS can already infer the type of x from game.distributions?

It was a solution to

ERROR in src/model/game/GameManager.ts:38:59 TS7006: Parameter 'x' implicitly has an 'any' type. 36 | // Map distribution values onto the r2mm StorePlatformMetadata type. 37 | // The `identifier` property may not be present, so we check it beforehand to avoid JSON import lint errors. > 38 | const distributions = game.distributions.map((x) => new StorePlatformMetadata( | ^ 39 | GetStorePlatformFromName(x.platform), 40 | x.identifier! 41 | )

however I'm sure there's a better way to do things.

Again, my IDE currently infers the type (when no type is manually provider) in an awkward but correct looking fashion:

(parameter) x: { platform: string; identifier: string; } | { platform: string; identifier: string; } | { platform: string; identifier: null; } | { platform: string; identifier: null; } ``` Perhaps you did some changes afterwards that got rid of the problem?

src/r2mm/installing/profile_installers/ModLoaderVariantRecord.ts

anttimaki · 2025-01-22T11:05:24Z

src/r2mm/installing/profile_installers/ModLoaderVariantRecord.ts

-// Casting is done here to ensure the values are ModLoaderPackageMapping[]
-export type GAME_NAME = keyof typeof VARIANTS;
-export const MOD_LOADER_VARIANTS: {[key in GAME_NAME]: ModLoaderPackageMapping[]} = VARIANTS;
+const OVERRIDES: { [key: string]: [ModLoaderPackageMapping] } = {


Should it be ModLoaderPackageMapping[] instead of [ModLoaderPackageMapping]?

I think Record<string, [ModLoaderPackageMapping]> would be preferred way to achieve the same thing

src/r2mm/installing/profile_installers/ModLoaderVariantRecord.ts

anttimaki · 2025-01-22T11:12:10Z

Also related to some of @ebkr's comments and some of mine, we might want to refactor the enums in the r2mm codebase to a) be consistent b) use values that can be more easily matched with the values from ecosystem, while keeping them somewhat generic to avoid any weirdly specific names in the ecosystem that mean nothing outside the context of the mod manager.

If we decide to go that way, would probably make sense to do that before these changes?

This commit and PR largely aim to replace previously hard-coded game and mod definitions within R2MM with data read directly from the ecosystem schema (thunderstore-io/ecosystem-schema). Validation is accomplished through ajv, which allows us to validate the provided ecosystem-schema document with its cooresponding JSON schema (ecosystem.json and ecosystemJsonSchema.json).

ethangreen-dev · 2025-01-30T00:57:47Z

src/r2mm/installing/profile_installers/ModLoaderVariantRecord.ts


-const DEFAULT_MELONLOADER_MAPPING = [new ModLoaderPackageMapping("LavaGang-MelonLoader", "", PackageLoader.MELON_LOADER)];


This will need to be carried over somehow.

ethangreen-dev marked this pull request as ready for review January 21, 2025 08:43

ebkr reviewed Jan 22, 2025

View reviewed changes

anttimaki reviewed Jan 22, 2025

View reviewed changes

ethangreen-dev force-pushed the deep-ecosystem-integration branch from 255975e to 5a25f21 Compare January 29, 2025 23:34

ethangreen-dev force-pushed the deep-ecosystem-integration branch from 5a25f21 to be1215a Compare January 29, 2025 23:55

ethangreen-dev commented Jan 30, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deep ecosystem integration #11

Deep ecosystem integration #11

ethangreen-dev commented Jan 13, 2025

ebkr left a comment •

edited

Loading

ebkr Jan 22, 2025

ethangreen-dev Jan 22, 2025

anttimaki left a comment

anttimaki Jan 22, 2025

ethangreen-dev Jan 22, 2025

anttimaki Jan 22, 2025

ethangreen-dev Jan 22, 2025

anttimaki Jan 22, 2025

anttimaki Jan 22, 2025

ethangreen-dev Jan 22, 2025

anttimaki Jan 23, 2025

anttimaki Jan 22, 2025

ethangreen-dev Jan 22, 2025

anttimaki Jan 23, 2025

anttimaki Jan 22, 2025

anttimaki commented Jan 22, 2025

ethangreen-dev Jan 30, 2025


		const DEFAULT_MELONLOADER_MAPPING = [new ModLoaderPackageMapping("LavaGang-MelonLoader", "", PackageLoader.MELON_LOADER)];

Deep ecosystem integration #11

Are you sure you want to change the base?

Deep ecosystem integration #11

Conversation

ethangreen-dev commented Jan 13, 2025

ebkr left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anttimaki left a 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

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

anttimaki commented Jan 22, 2025

Choose a reason for hiding this comment

ebkr left a comment •

edited

Loading