boozook / playdate

Playdate Build Tools and API
https://mastodon.gamedev.place/@pd
MIT License
133 stars 8 forks source link

Manifest fields override for cargo dev targets (e.g. examples) #354

Closed boozook closed 3 months ago

boozook commented 4 months ago

Could be great to get option to set (override) manifest's fields specially for concrete example. BundleID is mostly important.

Related message in the matrix room by @paulstraw.

paulstraw commented 4 months ago

Thanks for considering this. The name would also be nice, because otherwise all the cards on device will still be indistinguishable by eye.

boozook commented 4 months ago

I see this as table in package.metadata.playdate.dev."{example-name}" with same manifest fields as in main table, but all fields should be optional there.

And I see consistency problem here. Look, we have package.metadata.playdate.dev-assets that inherits by package.metadata.playdate.assets. To make this all consistent, it should be look like one of following two options:

  1. just add new fields
    • package.metadata.playdate.{manifest fields} - manifest
    • package.metadata.playdate.assets - main package assets
    • package.metadata.playdate.dev-assets - for all examples, inherited by main assets
    • new: package.metadata.playdate.dev.{example-name}.{manifest fields} - example's manifest, inherited by main manifest
    • or may be: package.metadata.playdate.dev-{example-name}.{manifest fields} to look like dev-assets
  2. change existing structure (also change toml scheme 😬), add new fields
    • package.metadata.playdate.{manifest fields} - manifest
    • package.metadata.playdate.assets - main package assets
    • change: package.metadata.playdate.dev.{example-name}.assets - for all examples, inherited by main assets
    • new: package.metadata.playdate.dev.{example-name}.{manifest fields} - example's manifest, inherited by main manifest

I prefer first one because it's simpler with less changes and without breaking changes. But second option is more beautiful and consistent.

Also, this ☝🏻 + #209 together could look pretty awesome ❤️‍🔥

paulstraw commented 4 months ago

This makes sense to me. For my particular use case, having the ability to specify assets at the individual example level would be great, but definitely not as important as the other metadata.

If pursuing the second route, maybe package.metadata.playdate.dev-assets could still be supported (deprecated?) to avoid having it be an immediate braking change?

I'd be happy to help out with a PR if you'd like, though I'd need a bit of a nudge in the right direction since I'm new to the codebase (and Rust itself).

boozook commented 4 months ago

🤔 Let's say, okay. But here's another open question: One powerful cargo-playdate feature that may become main-killer-feature in the future is compiling optimized binaries without gcc. ([[bin]]) There can be a lot of binaries. It turns out that for each of them we should be able to set manifest fields too.

boozook commented 4 months ago

Also I have issue about workspace metadata and inheritance like package-fields - #26.

boozook commented 4 months ago

@paulstraw, I'll do a little refactoring before. Because there is a lot of unnecessary stuff left there, e.g. for cargo-metabuild support and other currently unnecessary junk. That's easy and fast 🤔

boozook commented 4 months ago

@paulstraw I don't think this whole thing is a good idea. The complexity of the system grows catastrophically. It is no longer a chain of inheritance, but a reverse graph, where each node has several parents (read as sources).

Why don't we just name examples by the name of the example ([[example]].name or filename instead of package name?

paulstraw commented 4 months ago

That's fair enough. It doesn't really matter to me how the configuration happens, as long as it's possible to provide custom configuration for examples. As long as the bundle ID and name can be changed for examples, I agree that the main issue will have been resolved.

boozook commented 4 months ago

@paulstraw: In the #360 is a fix that you waiting for. But that's a temporary solution and point for change in future. So, I'm not closing this issue until there is a full implementation of the feature.

boozook commented 3 months ago

360 published as beta now. I'll close this issue if all is ok there.

boozook commented 3 months ago

@paulstraw, do you have any comments or remarks on the implementation?