Skip to content

Feature proposal: extensible content icons #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
agc93 opened this issue Aug 7, 2020 · 1 comment
Open

Feature proposal: extensible content icons #2

agc93 opened this issue Aug 7, 2020 · 1 comment

Comments

@agc93
Copy link
Contributor

agc93 commented Aug 7, 2020

I was poking around with this but wanted to check if you'd be interested in a feature before I sink too much time into a PR. The current implementation means that content icons are only possible for the predefined types in the typeDescription object in this extension.

I started mucking around with making that extensible so that other extensions could conceivably add their own detectable types and icons that would be shown by the "standard" table column from this extension. Would this be something you'd consider including?


What I started with basically came down to this:

  • Load extra type descriptions from the session state during ModContent's render()
  • Check for and load (if present) an extra attribute (content-extra in my PoC) from the current mod
  • If a type in the attribute included a colon (could be any delimiter really), then assume that's the icon set's name, defaulting to the existing mod-content.

Alternatively with a bit of retooling I'm sure it would be possible to reuse the same approach using the same attribute to avoid duplication


I don't even know if it was possible but figured I'd check if it was something you'd consider before I sink too much time into it

@TanninOne
Copy link
Contributor

Generally I try to keep extensions as self-contained (independent from other extensions) as I can, simply so that changing one extension doesn't break others.

You could add something like this:
context.registerModContentType = (id, name, icon, iconset) => ...
context.registerModContentDetection = (typeId, extension, condition?) => ...
that let any extension add their own content detection, but then as soon as extension start using these, you can't change that api (and, in extension, the functionality of mod-content) any more without considering extensions that may already be using it.
The result is either the development of an extension that exposes an api is stifled or - if the api gets changed without care - a lot of broken extensions.

Obviously we already have this problem with the core application but there we already have rules that there can't be breaking changes in the api outside major updates and we do have to be super-careful to not break backwards compatibility.

My point is: adding an api to an extension is different from adding a feature, it shouldn't be "gimmicky".

In this case I see how this would be useful, if you were to develop this api (assuming it works) I'll merge it, but my original plan for mod-content was simply to accept PRs that add file types for games, so that all the content detection was in one place.

More concretely: afaict your proposal wasn't to add detection for content based on file extension and such but for the other extension to just set a content flag directly and the mod-content extension to use it.
That is an interesting feature. Right now the extension doesn't allow the content to be set programmatically, we just cache the type detected based on file extension and a "condition" function.
If you add this, I think the content-extra flag should still just contain the abstract type, e.g. "script" and then if you need further types those should be added with a "register..." function.

In case you didn't know: you can simply add register functions to the context object, they will automatically become available to all extensions. Extensions can call "context.optional.register...", if said register function doesn't exist (e.g. because the extension isn't installed) the call will simply be skipped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants