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

Allow resources to be loaded from files #824

Closed
wants to merge 11 commits into from

Conversation

cspiegel
Copy link
Contributor

This allows both Hugo and Adrift games to load media without the clumsy method of writing out SND and PIC files.

@curiousdannii
Copy link
Contributor

curiousdannii commented Feb 26, 2024

I had been thinking that there might need to be resources from both a blorb and an extra resource file, but that won't be the case with any of our terps, will it? That makes things easier.

It would be simpler if garglk_add_resource_from_file just returned the ID, with 0 meaning a failure. Any reason why the IDs couldn't just start with 1? (So subtract 1 when accessing garglk_image_data.)

Please add a new #define GLK_MODULE_ and use that instead of GARGLK in the terps. Because I'll definitely want to implement this too.

Does it look for filename as a sibling to the storyfile, or in the current working directory? I couldn't tell from the code (not familiar enough with C++), but I think it needs to be the former. I assume all filenames would be just bare filenames, but perhaps it would be worth adding a guard check that it doesn't have a slash?

Otherwise this looks good!

Probably worth asking for feedback on the forum too though.

And then, to add support to TADS. Eventually...

@cspiegel
Copy link
Contributor Author

I had been thinking that there might need to be resources from both a blorb and an extra resource file, but that won't be the case with any of our terps, will it? That makes things easier.

Agreed: this is really for formats that don't use Blorb.

It would be simpler if garglk_add_resource_from_file just returned the ID, with 0 meaning a failure. Any reason why the IDs couldn't just start with 1? (So subtract 1 when accessing garglk_image_data.)

That's reasonable. I can make the switch.

Please add a new #define GLK_MODULE_ and use that instead of GARGLK in the terps. Because I'll definitely want to implement this too.

Thanks, I completely forgot about that; on that note, where do you land on the naming, then? I really don't want to step on the GLK/glk namespace at all, but I'm 100% fine with marking it as something more generic than just Gargoyle.

Does it look for filename as a sibling to the storyfile, or in the current working directory? I couldn't tell from the code (not familiar enough with C++), but I think it needs to be the former. I assume all filenames would be just bare filenames, but perhaps it would be worth adding a guard check that it doesn't have a slash?

The interpreters provide the full path; I thought that made more sense because they're in more of a position to know where the resource file/files are. Everything I've seen always has the resources in the same directory for SCARE (for Hugo the resource file is always the game file), but this is a bit more flexible, and it's possible that SCARE games might have resources elsewhere.

I'm not sure it's necessary to worry about opening arbitrary files: they're opened read-only, and if the user has access to the file, there's no harm, and if they don't, it'll just fail to load.

Probably worth asking for feedback on the forum too though.

Good point; I'll get a post up shortly.

Thanks for all the input!

@curiousdannii
Copy link
Contributor

Thanks, I completely forgot about that; on that note, where do you land on the naming, then? I really don't want to step on the GLK/glk namespace at all, but I'm 100% fine with marking it as something more generic than just Gargoyle.

garglk_ is fine, that's what we're basically stuck with for garglk_set_zcolors and the others.

The interpreters provide the full path; I thought that made more sense because they're in more of a position to know where the resource file/files are. Everything I've seen always has the resources in the same directory for SCARE (for Hugo the resource file is always the game file), but this is a bit more flexible, and it's possible that SCARE games might have resources elsewhere.

Oh, if the terps provide a full path then that makes things simple on the Glk side.

I think you've mixed it up again, as it's Hugo that can have multiple files (as can TADS). But for Scare, if it can only refer to the storyfile, which is already open, do you think it would be good to add a second function for adding resources from memory? Or do you think it wouldn't make enough of a performance difference to bother with?

@cspiegel
Copy link
Contributor Author

I think you've mixed it up again, as it's Hugo that can have multiple files (as can TADS). But for Scare, if it can only refer to the storyfile, which is already open, do you think it would be good to add a second function for adding resources from memory? Or do you think it wouldn't make enough of a performance difference to bother with?

Haha, I think I'm perpetually condemned to mix those two up.

I don't think it's worth adding the memory functions. At least for Scare (yes, really Scare), I'm not even sure if there's a full buffer of the story file. I can see that it reads in the header first, and then eventually the rest of the file, though I'm not sure if it rewinds first. In any case, the file will almost certainly already be cached by the OS, so rereading it will be effectively free. And if not, yeah, it's really not going to make a performance difference anyway. And in this PR, I have it caching loads, so resources will be loaded at most once, and then pulled from memory afterward.

@cspiegel
Copy link
Contributor Author

Alright, I've decided to split this into two parts. First is the API implementation (now in #825), and when that's complete, I'll do the Hugo/Scare updates.

@cspiegel
Copy link
Contributor Author

Closing in favor of #825.

@cspiegel cspiegel closed this Feb 29, 2024
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