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

Adjust socket names to match spec, adjust GLB import and export to match spec #27

Open
hybridherbst opened this issue Nov 13, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@hybridherbst
Copy link
Contributor

hybridherbst commented Nov 13, 2024

In the spec, a lot of sockets use "value" as name, but in this repo it seems its mostly "val":
https://github.com/KhronosGroup/glTF/blob/interactivity/extensions/2.0/Khronos/KHR_interactivity/Specification.adoc#36422-pointer-set

See also #24 (comment)

Also the nested "graph" object should be dropped – it's not used in the current spec:

image
@hybridherbst
Copy link
Contributor Author

Hey @mattmacf98, I made several fixes and improvements here:
https://github.com/needle-tools/glTF-InteractivityGraph-AuthoringTool/commits/dev/

  • added a partial implementation of KHR_node_visibility (needs Babylon 7 for a full implementation, which already has the extension already either ways)
  • hardened the graph layouting (some cases with exceptions at the moment)
  • adjusted import/export to not assume the KHR_interactivity.graph.nodes object
  • fixed sequence nodes not working with arbitrary input names
  • fixed flow/delay to use "done", was "completed"
  • fixed pointer/get and pointer/set to use "value" instead of "val"

Let me know if you want individual PRs for them. But you can also cherry-pick things out of there if that's easier.

@mattmacf98
Copy link
Contributor

Thank you so much! I just read through all of the changes and they look good to me, would you be able to open a single PR for all of these together? If not, I can just go through and cherry pick them

@hybridherbst
Copy link
Contributor Author

hybridherbst commented Nov 13, 2024

Sure, I did that here now:

As mentioned there this is partial – it would probably be good to do a pass over the naming of things but I didn't get to that yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants