cspiegel / terps

4 stars 2 forks source link

Alan2 save prompt other than Garglk #19

Open cibersheep opened 2 years ago

cibersheep commented 2 years ago

I've been poking the code and made save prompt work for remglk adding this https://github.com/cibersheep/terps/commit/0ce603856c9a5ca66e10aecd83e39a0c18b1f69c

It needs additional macros in the cmake, when building alan2 it need GLKUNIX_FILEREF_GET_FILENAME This works for remglk, as mention but it fails for glkterm. I'm assuming then, it may be a wrong approach. Should I add a condition on the cmake? or do you reckon the whole approach is wrong?

cspiegel commented 2 years ago

The GLKUNIX_FILEREF_GET_FILENAME should only be defined by the Glk implementation, as it signals that a specific API extension is available. Or, possibly, conditionally defined in the Alan code if running under Gargoyle, but not in CMakeLists.txt, because then it will fail on Glk implementations which don't suppose this API function.

This looks like something Zarf added to remglk as a more standard version of Gargoyle's garglk_fileref_get_name. As such we'll want to add this to Gargoyle: I've got a new issue at garglk/garglk#629.

But for the immediate issue at hand, if I'm understanding right, you want to use this function regardless of whether it's the Gargoyle or new "standard" glkunix name, correct? You also have to consider a third possibility of the function not existing at all. So something like this, maybe:

#include "glk.h"
#if defined(GLK_MODULE_FILEREF_GET_NAME) && !defined(GLKUNIX_FILEREF_GET_FILENAME)
#define glkunix_fileref_get_filename garglk_fileref_get_name
#define GLKUNIX_FILEREF_GET_FILENAME
#endif

Now in the rest of the code you use GLKUNIX_FILEREF_GET_FILENAME and glkunix_fileref_get_filename. This will work with the current garglk as well as any future releases which switch over to the new names.

curiousdannii commented 2 years ago

You should test for GLK_MODULE_FILEREF_GET_NAME for the presence of the garglk function.

cspiegel commented 2 years ago

You should test for GLK_MODULE_FILEREF_GET_NAME for the presence of the garglk function.

Right, good call, updated.

cibersheep commented 2 years ago

Thank you to take the time to explain