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

AppConfig extensible custom properties API #380

Closed
nanodeath opened this issue Apr 27, 2021 · 1 comment · Fixed by #381
Closed

AppConfig extensible custom properties API #380

nanodeath opened this issue Apr 27, 2021 · 1 comment · Fixed by #381
Labels
feature New functionality for the program

Comments

@nanodeath
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Most configuration in Zircon is done through the AppConfig object (and its buddy, AppConfigBuilder). However, all the configuration here is generic -- applicable to all (or most) Zircon programs.

However, there's some configuration that's going to be specific to a particular extension, platform, or implementation, and it'd be convenient for that to still be managed via the central AppConfig object. So, we need some kind of API that's exposed to plugin authors and, inevitably, minimally exposed to end-users.

Describe the solution you'd like

I'm proposing the following new APIs:

AppConfigBuilder

fun <T> withProperty(key: AppConfigKey<T>, value: T): AppConfigBuilder

AppConfig

fun <T> getProperty(key: AppConfigKey<T>): T

AppConfigKey<T> would be a simple interface that looks like this:

interface AppConfigKey<T>

In both cases, properties are represented internally in a simple map.

Usage

These APIs would primarily be used by plugin authors and non-core code. For example, to support custom tileset loaders (which are specific to a particular rendering module), the zircon.jvm.swing module could contain these extension functions:

internal object SwingTilesetLoaders : AppConfigKey<List<TilesetLoader<Graphics2D>>>

fun AppConfigBuilder.withCustomTilesetLoaders(vararg tilesetLoaders: TilesetLoader<Graphics2D>) =
    withProperty(SwingTilesetLoaders, tilesetLoaders.toList())

internal fun AppConfig.getCustomTilesetLoaders(): List<TilesetLoader<Graphics2D>> = 
    getProperty(SwingTilesetLoaders).orEmpty()

As you can see, the end user is easily able to set the custom tileset loaders and (if desired) the ability to retrieve them is limited to the library author.

Describe alternatives you've considered

  • Make AppConfig open and provide subclasses with the necessary methods. However, this means the end user can only pick a single specific subclass, making supporting multiple extensions impossible.
  • We could create additional AppConfig-like objects that get passed into the main Zircon constructors, but this seems needlessly complicated.

End users would have little use for this API, so we should dissuade them from using it.

Additional context
This is a blocker for custom tileset loaders.

nanodeath added a commit to nanodeath/zircon that referenced this issue Apr 27, 2021
This enables plugin authors to easily set and retrieve configuration options
for the end developer.

Closes Hexworks#380.
@adam-arold adam-arold added enhancement Enhances existing functionality without introducing new features feature New functionality for the program and removed enhancement Enhances existing functionality without introducing new features labels Apr 29, 2021
@adam-arold
Copy link
Member

I dig this AppConfigKey approach! It keeps type safety while retaining the flexibility of a Map. I've added this to the board!

nanodeath added a commit to nanodeath/zircon that referenced this issue Apr 30, 2021
This enables plugin authors to easily set and retrieve configuration options
for the end developer.

Closes Hexworks#380.
@adam-arold adam-arold added this to the 2021.1.0-RELEASE milestone Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality for the program
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants