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

Repair command replacement in Glulx Entry Points #66

Closed

Conversation

EvanBalster
Copy link

This PR restores some long-lost functionality for glulx command replacement in Glulx Entry Points. Compatibility with existing extensions ought to be unaffected.

This restores the function of some dependent extensions such as the original Inline Hyperlinks by Daniel Stelzer. (I have learned that Stelzer's extension has been supplanted by alternatives, but I was in too deep by that point!)

The problem is detailed here: #65

Glk Events is modified as follows:

  • A new top-level rulebook called the glk event processing rules is added to Glk Events.
  • That rulebook contains a compatibility rule that abides by the existing glulx input handling rules...

Glulx Entry Points (abbreviated GEP) is modified as follows:

  • The compatibility rule in Glk Events is disabled when both extensions are included.
  • A new glk event processing rule invokes GEP's glk event handling function and stashes the event result.
  • HandleGlkEvent is implemented by retrieving and returning the stashed event result.

@curiousdannii
Copy link
Member

I don't understand why you needed to add a new rulebook. Could you just add this rule instead?

A glulx input handling rule (this is the glulx event handling rule):
	Now GEP internal glk event result is the value returned by glk event handling.

@EvanBalster
Copy link
Author

EvanBalster commented Apr 13, 2021

In my original fix I did just that. But there was a problem.

See this excerpt from Glulx Entry Points:

To decide what number is the value returned by glk event handling (this is the handle glk event rule):
	now glulx replacement command is "";
	follow the glulx input handling rules for the GEP internal current glk event;
	[...]

glk event handling follows the glulx input handling rules, which would need to include the rule that performs glk input handling. So we have a recursion problem.

The fix I attached in #65 attempts to get around the problem with a recursion-blocking flag, which works, but that approach is problematic for two reasons:

  • It absolutely requires new rule to be the first in the glulx input handling rulebook, so it can prevent all the others from running outside the glk event handling routine;
  • It breaks the reentrancy of the rulebook (in case a glk event is generated while inside a glulx input handling rule).

@curiousdannii
Copy link
Member

curiousdannii commented Apr 13, 2021

Hmm, I'll have to take another look at it all in detail. I don't know why it would be doing anything at risk of being recursive. Maybe two rule books is appropriate, but they should be in different extensions I think.

From the perspective of Glk Events, it intercepts the glk_select function, putting the actual result of the function through the glulx input handling rules (to do things like redrawing the screen), and then returns the value up to whatever called it. There could be circumstances when the caller wants to put the result through another rulebook, or it might just use the glk results directly (as the I7 template layer does.)

glk_select caller (could use a rulebook to process results)
    ↓                   ↑
---------(Glk Events Extension)---------
    ↓                   ↑
fake glk_select  →  input handling rules
    ↓    ↑
real glk_select

If this pattern is followed then there should be no risk of recursion.

I haven't investigated fully yet to see how this PR would differ from this patten.

@EvanBalster
Copy link
Author

EvanBalster commented Apr 13, 2021

(Apologies in advance for my odd terminology; procedural programmer here only recently reorienting with Inform.)

GEP's glk event handling function was originally was the caller of the glulx input handling rulebook. And indeed, GEP must follow this rulebook somehow in order to collect its outcome and replacement command.

When GEP was modified to depend on Glk Events, the glk event handling function was bypassed, breaking the command replacement features. With these changes, Glk Events now calls the old GEP function (via GEP's "glulx event handling rule") and the GEP function follows the old rulebook. In the event GEP is not used, Glk Events' new rulebook follows the old rulebook (via the compatibility rule).

It would be nice if the rulebooks could be declared in different plugins but I'm not sure if it's possible. The original problem had to do with the declaration of the glulx input handling rules being moved from Glulx Entry Points to Glk Events. Fixing the problem means GEP must follow the old rulebook as it did before, but Glk Events needs to follow it in the event GEP is not included.

Behavior between these two cases should be consistent (unless GEP outcomes or the glulx replacement command are used). In that case, the glk event handling function in GEP does nothing of interest after the rulebook is finished and cedes control back to Glk Events.

(Note that only one extension in this repository includes Glk Events but not GEP.)

Classic GEP:  GlkHandleEvent -> glk event handling -> glulx input handling rules ... command replacement

Current GEP:  (does not follow glulx input handling rules)
Current GE:   glk_select -> glulx input handling rules

Patched GE:   glk_select -> glk event processing rules -> compatibility rule* -> glulx input handling rules
Patched GEP:  glk event processing rule -> glk event handling -> glulx input handling rules ... command replacement & stashes result -> GlkHandleEvent

* unless GEP included

@EvanBalster
Copy link
Author

Anyway... That's a lot of chat but I kept the changes themselves as succinct as possible.

The detail I'm most uncertain about was whether it's appropriate to restore the GlkHandleEvent call. Is that obsolete or deprecated? I couldn't find another way to signal the continuation of player input vs. the submission of the pasted command. I also couldn't find any documentation about GlkHandleEvent itself or the appropriate return values.

@curiousdannii
Copy link
Member

You likely haven't seen this, but Glulx Entry Points has recently been effectively incorporated into the core code of Inform: ganelson/inform#76
As such I'm going to close this, though if someone is still using 6M62, and they're having trouble with GEP, then this could be useful for them to consider adapting.

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.

2 participants