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 loading of multimedia for JACL and Hugo #649

Merged
merged 2 commits into from
Jun 24, 2022

Conversation

cspiegel
Copy link
Contributor

@cspiegel cspiegel commented Jun 8, 2022

Fixes #648

@curiousdannii
Copy link
Contributor

It would be good to get the Hugo changes upstream. And do they all need to be gated behind GARGLK checks? I don't see any GARGLK specific functions.

@cspiegel
Copy link
Contributor Author

cspiegel commented Jun 9, 2022

It would be good to get the Hugo changes upstream. And do they all need to be gated behind GARGLK checks? I don't see any GARGLK specific functions.

A couple things...

First, the technically-not-standard glkunix_stream_open_pathname function is used, meaning this is limited to Glk implementations which provide a glkunix startup, which, to be fair, is most of them. This could be rewritten to use standard I/O routines, but the way the interpreter is written means that's a bit invasive: macros with the same name as standard I/O functions are defined as wrappers around Glk functionality, presumably because the interpreter originally was written to use standard I/O routines only, and Glk support was retrofitted via these wrappers. Anyway, it makes a "proper" fix more difficult. For now the easy fix is using this glkunix function, but I'm not sure how upstream would prefer that be handled, since it's not a standard Glk function. Maybe it's enough to just say "we support Unix startups only". I don't know. The alternative, I suppose, would be requiring a macro to be set at build time which is just pushing the work onto the person doing the building, but then, that person knows which Glk implementation is being used, so it's not a huge burden (this is how I do it with Bocfel).

The next part, though, is tied to specific Glk implementations. Multimedia support is very hacky: pictures/sounds are extracted from the Hugo resources and written directly to files with names like PIC0 and SND0, which Gargoyle is able to load if there's not a Blorb file associated with the game. xglk supports these PIC files, though I'm not sure any others do... This is a "suggested" method by the Blorb specification, but I don't know how appreciated it'd be if the Hugo interpreter dumped these files next to every Hugo game regardless of whether the Glk implementation supported them. In all honesty, though, I'm not sure any other Glk implementations can currently use Hugo multimedia resources, which means it may indeed make sense to hide these behind GARGLK, which would then also mean that the code using glkunix_stream_open_pathname would still want to be protected by GARGLK as well, since there's no use in even looking at the resource files if they can't even be used.

I admit I wasn't thinking about upstream when I hacked this solution together, and there is one thing that I'll definitely clean up for that, and just because it's the right thing to do: the new hugo_path_to_game variable isn't declared in any headers, but instead directly extern'd from source files. This is awful practice and I shouldn't have done it. So even if it never gets upstreamed, I'll fix it anyway, because it's not cool.

I think I'll probably have to open an issue upstream to forge a path forward there. For now I'll leave this pull request in stasis, but one way or another I'll ensure the fix gets in soonish, to try to maybe push out a 2022.1.2 release.

@curiousdannii
Copy link
Contributor

curiousdannii commented Jun 9, 2022

Ahh, looks like glkstart.h doesn't have its own header to rely on? Maybe, like Bocfel, the terps should have a GLK_UNIX define then. And if someone wants to build it against Windows Glk for example, then they can add their own parallel solution.

pictures/sounds are extracted from the Hugo resources and written directly to files with names like PIC0 and SND0, which Gargoyle is able to load if there's not a Blorb file associated with the game. xglk supports these PIC files, though I'm not sure any others do... This is a "suggested" method by the Blorb specification, but I don't know how appreciated it'd be if the Hugo interpreter dumped these files next to every Hugo game regardless of whether the Glk implementation supported them. In all honesty, though, I'm not sure any other Glk implementations can currently use Hugo multimedia resources, which means it may indeed make sense to hide these behind GARGLK, which would then also mean that the code using glkunix_stream_open_pathname would still want to be protected by GARGLK as well, since there's no use in even looking at the resource files if they can't even be used.

Ah, yeah that's quite hacky. I think a better approach could be to construct an in-memory blorb. Though I haven't tried it.

@cspiegel
Copy link
Contributor Author

I've got an issue open upstream at hugoif/hugo-unix#4, and if that gets resolved, I'll update from there. For now, though, ensure loading is fixed locally for future releases.

@cspiegel cspiegel merged commit 7210ae8 into garglk:master Jun 24, 2022
@cspiegel cspiegel deleted the glk-files branch June 24, 2022 00:18
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.

Multimedia broken in JACL and Hugo
2 participants