extensions icon indicating copy to clipboard operation
extensions copied to clipboard

Repair command replacement in Glulx Entry Points

Open EvanBalster opened this issue 3 years ago • 5 comments

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: https://github.com/i7/extensions/issues/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.

EvanBalster avatar Apr 13 '21 04:04 EvanBalster

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.

curiousdannii avatar Apr 13 '21 04:04 curiousdannii

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 https://github.com/i7/extensions/issues/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).

EvanBalster avatar Apr 13 '21 07:04 EvanBalster

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.

curiousdannii avatar Apr 13 '21 08:04 curiousdannii

(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 avatar Apr 13 '21 19:04 EvanBalster

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.

EvanBalster avatar Apr 13 '21 20:04 EvanBalster