Strange interaction between Flexible Windows and end the story phrase

Authoring Inform 7
1

I have the following convenience phrases:

to end the story with (t - text):
	say "[line break]Press any key to continue.[paragraph break]";
	Maybe Wait;
	end the story saying t;

To Maybe Wait:
	if DEBUG is false:
		wait for any key;

Sometimes it gives me wacky output:

Press any key to continue.

*** #FFFFFF ***

My guess is that the memory used to store t (or rather finale below) is being overwritten.

To end the story saying (finale - text)
	(documented at ph_endsaying):
	(- deadflag={-by-reference:finale}; BlkValueIncRefCountPrimitive(deadflag); story_complete=false; -).

but why, and/or how to prevent it I don’t know. I’ve tried putting t in a temporary value but that doesn’t fix it. I thought it was my use of a flexible window (I’m using 6M62), but that doesn’t seem to be the issue… etc.

It seems to work when I just duplicate the code everywhere instead of using a phrase, but I haven’t tested that with all of the different story ends.

Any idea what’s going on?

2

I seem to recall something about this issue popping up because there was no permanent reference to the text that would be used.

Would assigning the text to be used to a global help?

3

Can you show us where you’re invoking “end the story with”?

4

It seems to in the places I’ve tried it in my WIP, but it’s always been intermittent, so I’m not sure if it’s really a fix, or just served to stir the pot so to speak.

5

Trying to find a minimal example, but so far I can’t reproduce the behavior outside of my WIP.

6

Aha! I think I found the problem. The #FFFFFF was leaking from Flexible Windows. What it looks like is that I was using

close the notes window;
open the notes window;

to force refresh, when I should have been using refresh the notes window. :man_facepalming:

I’m going to assume this is the problem for now until I see anything different. Thanks!

7

Hmm, no. It seems to happen whenever you open a text window before calling end the story with. Still investigating…

8

In the most simple version of your phrases above, I’m not seeing the same problem in 6M62.

My guess would be that the reference being passed is to a temporary value (which is a pointer to a location inside the stack) to the local variable t, and that other activity is rearranging the stack such that the reference is no longer valid for that text. It’s not obvious how that would be happening from just the code that you’ve shown, but the symptom seems similar.

If that is the problem, though, then assigning parameter t immediately to some global variable (e.g. ending text), and later using end the story saying ending text should solve it. Even if the stack has been disturbed, the reference to the global should still be valid.

9

I suspect you’re right. However I think I’ve found a maybe cleaner way to fix it. My code is closing and reopening a flexible window now and then; I’ve changed it to close the window by setting the height to 0, and that seems to work. The lesson being don’t open a window repeatedly; best do it in when play begins and leave it alone.

10

Even so, the fact that this is possible points to a bug in FW or Inform or both. So if you do manage to put together a minimal example, do let us know!

1 Like
11

Here’s the smallest example I can figure out to reproduce the problem. It involves some of my code, Flexible Windows, and a modified version of Stephen Granade’s Footnotes.

Include Flexible Windows by Jon Ingold.

[My code]

The notes window is a text buffer g-window spawned by the main window.
The position of the notes window is g-placebelow.
The scale method of the notes window is g-fixed-size.
The measurement of the notes window is 6.

The current note is a text that varies. The current note is "".

Rule for refreshing the notes window:
	say "---------------------------[line break]";
	say the current note;

Before printing the player's obituary:
	close the notes window; [forces refresh]
	if the current note is not "":
		open the notes window;
		refresh the notes window;

to end the story with (t - text):
	end the story saying t;
	
[extracted from Footnotes by Stephen Granade, with modifications by me.]
	
Footnote-name is a kind of value. The footnote-names are defined by the Table of Footnotes

Current footnote is a number that varies. The current footnote is 1.

Footnotes are a thing. Footnotes can be on or off. Footnotes can be given repeatedly or given only once. 

A footnote-name has a number called the footnote-index. The footnote-index of a footnote-name is usually 0.
A footnote-name has a truth state called the footnote-read. The footnote-read of a footnote-name is usually false.

footnotes are on.

Table of Footnotes
Name	Note	Necessary Space
footnote-placeholder	"barfoo"	2

Total necessary space is a number that varies. Total necessary space is 0.

[A forced-note is printed whether or not it's been shown before. A regular note is only printed if it's not been shown befeore.]
To say a/the/-- forced-note (fn - a footnote-name): 
	if footnotes are on:		
		if footnote-index of fn is 0:
			now the footnote-index of fn is the current footnote;
			increment the current footnote;
		say "[bracket][footnote-index of fn][close bracket]";
		if current note is "":
			now the current note is  "[bracket][footnote-index of fn][close bracket] [note of fn]";
		otherwise:
			now the current note is "[current note][line break][bracket][footnote-index of fn][close bracket] [note of fn]";
		now total necessary space is total necessary space + necessary space of fn;
		if the necessary space of fn > 0:
			now the measurement of the notes window is total necessary space + 1;
		otherwise:
			now the measurement of the notes window is 6;


To say a/the/-- note (fn - a footnote-name): 
	if expanding text for comparison purposes:
		stop;
	if footnotes are on:
		if footnotes are given only once: 
			if footnote-read of fn is false, say forced-note fn;
		otherwise:
			say forced-note fn.

[example code]

The lab is a room. 

Instead of jumping:
	say "X[note footnote-placeholder][paragraph break]";
	end the story with "foobar";
	
test me with "jump".
1 Like
12

I’ve reproduced this behavior in situations in my WIP (though not any minimal examples) that convinced me that the behavior is only due to Flexible Windows and nothing else I’m doing. I managed to narrow it down (using the fact that the erroneous text was #FFFFFF) to the following rule in Flexible Windows: (I managed to change the erroneous text by editing the following code, as additional evidence)

After constructing a textual g-window (this is the Gargoyle window padding rule):
	let T be the background color of the acting main window;
	if T is empty:
		let T be "#FFFFFF";
	set the Gargoyle window padding to T;

which uses

To set the Gargoyle window padding to (T - a text):
	set the background color of wintype 3 for normal-style to T;

So I guess the theory would have to be that something is going wrong here:

To set the background color of (win - a g-window) to (T - a text):
	(- glk_window_set_background_color( {win}.(+ ref number +), GTE_ConvertColour( {-by-reference:T} ) ); -).

I don’t really know how Inform 6 works, but here’s the conflicting code:

To end the story saying (finale - text)
	(documented at ph_endsaying):
	(- deadflag={-by-reference:finale}; BlkValueIncRefCountPrimitive(deadflag); story_complete=false; -).

Is there some reference counting thing going wrong here?

1 Like
13

As you note, Flexible Windows has:

After constructing a textual g-window (this is the Gargoyle window padding rule):
	let T be the background color of the acting main window;
	if T is empty:
		let T be "#FFFFFF";
	set the Gargoyle window padding to T;

This translates to:

[ KERNEL_4 
	tmp_0 ! Let/loop value, e.g., 'T': text
	;
	if (((((parameter_value ofclass K16_g_window) && ((~~Adj_44_t1_v10(parameter_value))))))) { ! Runs only when pattern matches
	if (debug_rules) DB_Rule(R_965, 965);
	! [2: let t be the background color of the acting main window]
	tmp_0 = I7SFRAME; 
			BlkValueCopy(tmp_0, GProperty(10, (Global_Vars-->15),p23_background_color));
	! [3: if t is empty]
	if ((((Adj_17_t1_v14(tmp_0)))))
	{! [4: let t be ~#FFFFFF~]
	    BlkValueCopy(tmp_0, TX_L_56);
	    }
	! [5: set the gargoyle window padding to t]
	(PHR_963_r15 (BlkValueCopy((I7SFRAME+WORDSIZE*2), tmp_0)));	! <-- here
	} else if (debug_rules > 1) DB_Rule(R_965, 965, 'action');
	rfalse;
];

The line that I marked seems to be one that is overwriting the stack frame value. With some debugging output added inserted manually into the generated I6, the >TEST ME output is:

>[1] jump
X[1]

<KERNEL_1 I7SFRAME = 537534>				<-- where "foobar" is stored, via "end the story saying..."
<KERNEL_4 I7SFRAME = 537526>
<KERNEL_4 tmp_0 = 537526>
<KERNEL_4 I7SFRAME+WORDSIZE*2 = 537534>		<-- where "#FFFFFF" is stored, via "set the Gargoyle window padding to..."


	*** #FFFFFF ***

Your example is fixed by storing the desired ending text in a global:

endingtext is a text that varies.

to end the story with (t - text):
	now endingtext is t;
	end the story saying endingtext;

EDIT: For more detail, the end the story saying... phrase doesn’t actually cause the story to end immediately at that point – it just sets the value of deadflag, which is later examined during the main game loop. Any non-false value triggers the shutdown rules, which is where the actual ending text will be printed.

The end the story saying... phrase takes the argument for assignment to deadflag by reference (though arguably it shouldn’t, given this issue). The reference in your case is to a local (i.e. stack-stored) variable.

I don’t think there’s any way to tell an I7 phrase that it should take its argument by reference, but if there were, then telling your end the story with... phrase to do that might be a better way to fix the issue.

3 Likes
14

Yeah, I would call this a bug. Certainly I7 authors don’t expect this difference between local and global variables! Normally, I7 makes sure to copy any locals that go out of scope if a reference to them is still around. It just misses this one because the reference is copied at the I6 level.

Making “end the story saying…” pass by value instead should fix it. I’d forgotten about that behavior of deadflag.

2 Likes
15

@rileypb, if you really want to avoid having an extra global, you can try:

Include (-

! modification of TEXT_TY_Copy()
[ TEXT_TY_CopySBAlt to_bv from_bv;
	BlkValueCopySB2(to_bv, from_bv);
	if (to_bv-->0 & BLK_BVBITMAP_CONSTANTMASK) to_bv-->0 = PACKED_TEXT_STORAGE;
	return to_bv;
];

-).

To decide which text is by-value (T - text):
	(- (TEXT_TY_CopySBAlt({-new:text}, {T})) -).

to end the story with (t - text):
	end the story saying by-value t;

I think it might be safe to just modify TEXT_TY_Copy() in that way instead of creating a modified version of the routine, but it’s always hard to tell for sure…

@Draconis: I think maybe current behavior is partly informed by a bug report I filed long ago when trying to do something fancy (maybe something like to decide which text is ending text... combined with end the story saying ending text?).

I seem to remember the root cause of my issue then being handling by reference vs. by value, but those terms didn’t mean much to me at the time.

1 Like
16

Ack you said that before; I was convinced it didn’t work because of a typo in my code. D’oh!

Thanks!

1 Like
17

No problem – happy to help!

Come to think of it, even getting ending the story with... (the wrapper-like I7 phrase) to handle things by reference wouldn’t work if the author did something like

let T be "You lasted [turn count] turns that time.";
end the story with T;

since T will invariably have an on-stack address in that case.

2 Likes
18

Filed it on the bug tracker. We’ll see if anything comes of it!

2 Likes
19

Looking into this more closely, there’s a chain of events that cause this problem to emerge, and I’m not sure that altering end the story saying... really gets at the root issue.

The first key events are caused by the Instead of jumping... rule:

! No specific request
! Instead of jumping:
[ R_971 I7RBLK;
	@push I7SFRAME;
	StackFrameCreate(2);
	BlkValueCreateOnStack(0, TEXT_TY);	! <-- step 1
	I7RBLK = KERNEL_1();
	BlkValueFreeOnStack(0);
	@pull I7SFRAME;
	return I7RBLK; ! nothing
];

The R_971() code sets up a space for a block value (in this case a text) on the stack (step 1).

[ KERNEL_1 ;
	if ((((action ==##Jump) &&  (actor==player)))) { ! Runs only when pattern matches
	self = noun;
	if (debug_rules) DB_Rule(R_971, 971);
	! [2: say ~X[note footnote-placeholder][paragraph break]~]
	say__p=1;! [3: ~X~]
	ParaContent(); print "X";! [4: note footnote-placeholder]
	ParaContent(); (PHR_970_r2 (I182_footnote_placeholder));! [5: paragraph break]
	ParaContent(); DivideParagraphPoint(); new_line; .L_Say3; .L_SayX3;! [6: end the story with ~foobar~]
	(PHR_968_r3 (BlkValueCopy(I7SFRAME, TX_L_55)));	! <-- step 2
	RulebookFails(); rtrue;
	} else if (debug_rules > 1) DB_Rule(R_971, 971, 'action');
	rfalse;
];

The subsidiary routine KERNEL_1() then uses that space on the stack as temporary storage for the argument (step 2) that will be passed to the routine compiled for the custom end the story with... phrase. It does this via…

[ BlkValueCopy to_bv from_bv  to_kind from_kind kovs;
	if (to_bv == 0) BlkValueError("copy to null value");
	if (from_bv == 0) BlkValueError("copy from null value");
	if (to_bv == from_bv) return;

	#ifdef BLKVALUE_TRACE;
	print "Copy: ", (BlkValueDebug) to_bv, " to equal ", (BlkValueDebug) from_bv, "^";
	#endif;

	to_kind = BlkValueWeakKind(to_bv);
	from_kind = BlkValueWeakKind(from_bv);
	if (to_kind ~= from_kind) BlkValueError("copy incompatible kinds");

	kovs = KOVSupportFunction(to_kind, "impossible copy");
	
	if (kovs(COPYQUICK_KOVS, to_bv, from_bv))
		BlkValueQuickCopyPrimitive(to_bv, from_bv, kovs);	! <-- always this case for TEXT_TY
	else
		BlkValueSlowCopyPrimitive(to_bv, from_bv, kovs, true);

	return to_bv;	! <-- step 3
];

which returns the value of the to_bv argument (step 3). This is the now-valid on-stack block value, which has been given the same underlying long block as from_bv, i.e. it is a “by value” copy (I think) of the text value being fed as a parameter to PHR_968_r3(), which is what’s generated for the new end the story with... phrase. The value being copied (a short block) is still a kind of reference, though, one which points to the long block holding the actual text information.

! Request 3: phrase text -> nothing
! to end the story with ( t - text ):
[ PHR_968_r3  
	t_0 ! Call parameter 't': text
	;
	! [2: end the story saying t]
	deadflag=t_0; story_complete=false;	! <-- step 4
	rfalse;
];

The I6 code for end the story saying... is injected directly into the I7 phrase (for end the story with...) that calls it. When the just-created on-stack short block value is passed to the end the story with... phrase, the I6 code does a direct pass-through assignment to deadflag. What’s being assigned is the to_bv address returned from BlkValueCopy(), which is still the same value as when it was passed as the to_bv argument to BlkValueCopy() and which was originally set up by R_971()/KERNEL_1(), i.e. Instead of jumping....

The proximal cause of rileypb’s issue is that the -by-reference: requirement for the end the story saying... phrase is making it copy the (only temporarily-valid) text object address it has been given to deadflag. However, it seems to me that maybe part of the fault is also the way that R_971()/KERNEL_1() is letting a pointer to a location on the stack “escape” the confines of the local routine. The short block of such a block value is guaranteed to be invalidated (even if not immediately overwritten) when the local stack frame is torn down on the way out of the routine, so it’s asking for trouble to hand out a pointer to something within it.

1 Like
20

I keep thinking about this issue.

Some additional factors contributing here:

  1. The compiler is basically blind to the nature of I6 code that is specified directly as in the end the story saying... phrase. It has no idea what deadflag is or what’s being done to it, and wouldn’t like it if it did know since the relevant code depends on weak typing.

  2. From the perspective of a particular routine, basically any incoming block value could be invalidated by any outgoing function call – there are no guarantees.

I’m wondering if it would make sense for a routine receiving block value A to create an A’ version for its own use (to ensure that a reference to the long block stays valid while it is running) but to only pass A (not A’, as seen here) to any outside routine that it calls. The routine can then wash its hands of the issue of maintaining the validity of A and also safely dispose of A’ itself. If every routine does that, then each should always have proper access to AsubL (the underlying long block), right?