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

Add support for Auto Splitting to the c api. #355

Closed
wants to merge 15 commits into from

Conversation

DarkRTA
Copy link
Contributor

@DarkRTA DarkRTA commented Sep 9, 2020

This works in its current state but some stuff needs to be adjusted.


Todo:

  • Find a way to expose logging as right now logging just does not work.

  • Find a way to make auto splitting support optional.

  • Improve bindings generator to conditionally include definitions for the auto splitting feature. Stubbed out functions instead.

@DarkRTA DarkRTA mentioned this pull request Sep 9, 2020
capi/Cargo.toml Outdated
@@ -5,7 +5,7 @@ authors = ["Christopher Serr <[email protected]>"]
edition = "2018"

[dependencies]
livesplit-core = { path = "..", default-features = false, features = ["std"] }
livesplit-core = { path = "..", default-features = false, features = ["std", "auto-splitting"] }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the reason why I didn't pull it in yet. The problem is that unconditionally depending on auto splitting will break for example the web version. So this needs to be an optional feature. However at least atm we don't support features in the binding generator. So either the C API needs to stub out the implementation or we need to improve our binding generator.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added the feature attribute to everything now, but I still need to work on improving the bindings generator.

That might take me a while since I'm not familiar at all with syn and binding generator's code is a bit messy.

@DarkRTA
Copy link
Contributor Author

DarkRTA commented Sep 14, 2020

The only thing really blocking this right now is the fact that we yet to improve the bindings generator. (All of the needed feature attributes have been set up.)

I've tried doing it myself but syn is being a pain in the ass to deal with and I cant seem to get it to do what I want. If anyone has any guidance on this I'd really appreciate it.

If you'd rather take a crack at it yourself, we are going to need some sort of --features parameter that takes in a list of features and skips generating bindings for stuff that isn't listed. This really long if statement is relevent here.


Edit: We should also probably handle logging but that's going to need further discussion.

@DarkRTA
Copy link
Contributor Author

DarkRTA commented Jan 25, 2021

I'm going to opt to just stub out the implementation here instead of extending the bindings generator, using this as an example.

We can improve the bindings generator at a later date.

@DarkRTA DarkRTA closed this Jan 8, 2022
@DarkRTA DarkRTA deleted the wasmtime-capi branch January 8, 2022 06:22
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.

3 participants