Skip to content
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

Don't use plugins as resources #10

Closed
alice-i-cecile opened this issue Aug 9, 2022 · 4 comments · Fixed by #19
Closed

Don't use plugins as resources #10

alice-i-cecile opened this issue Aug 9, 2022 · 4 comments · Fixed by #19

Comments

@alice-i-cecile
Copy link

Inserting the entire plugin as a resource is confusing and unidiomatic.

Instead:

  1. Merge FrameRateLimit and FrameRateLimitParam into a single type.
  2. Newtype WarnOnFrameDrop.
  3. Add FrameRateLimit and WarnOnFrameDrop as resources.
@aevyrie
Copy link
Owner

aevyrie commented Aug 9, 2022

confusing and unidiomatic.

What's the reasoning for this? To me it seems like the most discoverable way to handle symmetric config at insertion and runtime.

  • The config parameters are visible when you add the plugin
  • Those same parameters are accessible as a resource without needing to look up a secondary config type

If anything, this seems less confusing than configuring the plugin on insertion by instantiating the plugin struct, then accessing a completely different type if you want to change this configuration at runtime.

I agree this is unidiomatic, but I'm not convinced yet that it shouldn't be an idiom.

@alice-i-cecile
Copy link
Author

I can see your argument there! The main reason I find this confusing is that users tend to think of objects in terms of disjoint categories: components, resources, systems, plugins, entities. Blurring that line often results in friction and so I'm reluctant to do it without a very good reason.

My preference would be to ensure that each field represents a unique resource type: this allows users to clearly distinguish between "a plugin type" and a "resource type" while still making it easy to discover what the resource types are.

In this particular case, FrameRateLimit and FrameRateLimitParam are redundant, and solving that would fix a lot of the obfuscation.

@aevyrie
Copy link
Owner

aevyrie commented Aug 9, 2022

In this particular case, FrameRateLimit and FrameRateLimitParam are redundant, and solving that would fix a lot of the obfuscation.

Note, these are not redundant. FrameRateLimitParam is the user setting, whereas FrameRateLimit is the actual framerate limit the plugin will use. I.e. Auto isn't a framerate, it tells the plugin to detect the framerate on the fly, and use that.

@alice-i-cecile
Copy link
Author

Ah right, and you want to be continually checking that... That's harder.

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

Successfully merging a pull request may close this issue.

2 participants