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

Fix WASM font signature #1971

Merged
merged 3 commits into from
Jul 2, 2022
Merged

Fix WASM font signature #1971

merged 3 commits into from
Jul 2, 2022

Conversation

soxfox42
Copy link
Contributor

@soxfox42 soxfox42 commented Jul 1, 2022

I found another instance where the WASM function signature was incorrect. The line in linkTicAPI had only 8 arguments for font, the Zig and D bindings had 9, and the wasmtic_font implementation itself had 10. I've updated everything so that all 10 arguments are exposed everywhere. This shouldn't break existing binary compatibility, as the templates would already have been producing incompatible binaries when the font function was used.

I haven't been able to test either version fully, because I can't work out how to get the Zig template to compile at all, and the D template seems to still have a slightly incorrect function signature for font. Because of that, I'll leave this as a draft for now while I look into the issues further.

@soxfox42
Copy link
Contributor Author

soxfox42 commented Jul 1, 2022

The issue with D seems to have just been an issue with function signatures, specifically a missing const qualifier, and an incorrect type for three of the four transcolors arguments. I was able to build a simple cart using font after fixing those minor issues.

Zig, on the other hand, has a lot of weirdness going on. I worked out that it definitely doesn't build with Zig 0.9.1, only the master branch for now. Even after managing to get a build of that working on my system, the demo seems to be written for the raw API, and the user-friendly API seems to have some syntax errors. For now, I think the updated font signature should be fine, so I'll mark this pull request as ready, and move discussion of the Zig template back over to #1784.

@soxfox42 soxfox42 marked this pull request as ready for review July 1, 2022 04:53
@@ -63,12 +63,12 @@ void exit();
void elli(int x, int y, int a, int b, int color);
void ellib(int x, int y, int a, int b, int color);
bool fget(int id, ubyte flag);
int font(char* text, int x, int y, ubyte transcolors, int colorcount, int width, int height, bool fixed, int scale);
int font(const char* text, int x, int y, uint* transcolors, int colorcount, int width, int height, bool fixed, int scale, bool alt);
Copy link
Collaborator

@joshgoebel joshgoebel Jul 1, 2022

Choose a reason for hiding this comment

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

Are you SURE this is const char*? Is there a reason non-const values can't be used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really to be honest, I've never really done much with D. I added const because 1. that's what print and trace use and 2. it made my test compile.

I think const here just means the function won't be able to change the value, not that a constant value has to be passed.

@nesbox nesbox merged commit e6f233e into nesbox:main Jul 2, 2022
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