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

Added optional OS field for App Manifests #1605

Closed
wants to merge 2 commits into from
Closed

Added optional OS field for App Manifests #1605

wants to merge 2 commits into from

Conversation

aaronfranke
Copy link

@aaronfranke aaronfranke commented Jan 9, 2018

Fixes: #1587

However, this patch is incomplete, Itch debug log shows:

in manifest action [ActionNameHere], unknown field 'os' found

I don't know how to stop those error messages. I tried adding an entry to src/types/index.ts but it didn't work. I suggest you merge and apply the (hopefully simple) fix.

What this patch doesn't do is automatically start if only one option for your OS is available. Itch already launches if only one option in total exists.

case 1:
  return manifest.actions[0];

I'm not sure how to best implement that, but it's a low priority, because as you said, "if it doesn't [require actions besides "play"], you don't need a manifest at all".

manifest

This is what my test manifest looks like. Actions like "Red Alert" actually exist thrice, once for each OS, but only the action for the relevant OS appears. The "manual" action at the bottom does not contain an OS property, it only exists once in the manifest, and should show up everywhere.

Additionally, Itch will warn devs if the OS property contains something invalid.

Also, I have not tested this on MacOS, as I do not own a Mac.

I hope that cave-commands is an appropriate branch. I noticed that this is the branch you've been working on, so I figured I would push it there.

This patch is incomplete, Itch debug log throws `in manifest action [Name], unknown field 'os' found`
@fasterthanlime
Copy link
Collaborator

Hey, thanks for the PR!

cave-commands is the correct branch - unfortunately, the launch functionality (including manifest parsing, validation, interpretation) is currently being moved over to butler as part of #1530

That said, I think the approach here is sound - that's more or less how I was planning to do it :) And the docs part can be merged as-is I think.

(Note that all the install logic has already been ported to butler, see https://github.com/itchio/butler/blob/sz/cmd/operate/install.go - I'm currently working on the update logic, for which I'm taking some interesting detours - the launch logic is up next!).

At any rate, this feature is on the roadmap for v25, so it'll happen!

@fasterthanlime
Copy link
Collaborator

I'm working on #1642 right now, and I'm implementing this right now :)

Just thought I'd let you know because you always post careful issue reports & feature requests!

@aaronfranke
Copy link
Author

I'll close this PR now that everything's mostly done. Not gonna close #1587 until it's downstream, tho.

@aaronfranke aaronfranke deleted the cave-commands branch August 8, 2018 06:05
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 this pull request may close these issues.

None yet

2 participants