“Spacing issues” as a category are well-known to be Graham Nelson’s most-disliked kind of bug report, and there is often room for disagreement about what the rules should be. (It could be argued that the one with the extra line breaks is the “correct” behavior, for example.) That said, yes, this looks like an issue to me – there is certainly no good reason to expect that addition of an unrelated After rule will affect the spacing of the output in your example.
Coincidentally, I made a side note about a similar issue potentially affecting does the player mean rules in a post earlier today, and there are numerous places in template code where the same type of issue might pop up.
Unless a call to FollowRulebook() specifically requests suppression of line breaks, they will be generated by the machinery around the say__p global variable (and related variables). I don’t know all the details here, but basically say__p is a notice that something has been printed, and at various places (generally transitions between rules) a line break will be printed if it is set, and then it will be cleared.
The implication is that in your problem scenario something is leaving say__p set unexpectedly, but I haven’t tracked it down yet. [EDIT: I did track this down, to multiple causes. See expandable section below if you are interested in the minutiae.] Another way to deal with the issue is to stash the value of say__p temporarily while executing the After rulebook – this has the advantage that it won’t suppress breaks that would normally appear between After rules applying to NPC actions, a possible downside of the above approach.
[ CARRY_OUT_REQUESTED_ACTIONS_R rv;
if ((actor ~= player) && (act_requester)) {
@push act_requester; act_requester = nothing;
rv = BeginAction(action, noun, second);
if (((meta) || (rv == false)) && (deadflag == false)) {
if (FollowRulebook(UNSUCCESSFUL_ATTEMPT_RB) == false) {
CARRY_OUT_REQUESTED_ACTIONS_RM('A', actor); new_line;
}
}
@pull act_requester;
@push say__p; say__p = 0; ! ADDED
FollowRulebook(AFTER_RB, nothing, true);
@pull say__p; ! ADDED
ActRulebookSucceeds();
rtrue;
}
rfalse;
];
Just replace the function definition in the I6 inclusion if you want to use this version.
And now the promised minutiae:
Why this happens...
Why this happens…
Within the Rulebooks.i6t template file, one will find (with comments including “<–” added):
! ==== ==== ==== ==== ==== ==== ==== ==== ==== ====
! Rulebooks.i6t: Following
! ==== ==== ==== ==== ==== ==== ==== ==== ==== ====
Global process_rulebook_count; ! Depth of processing recursion
Global debugging_rules = false; ! Are we tracing rule invocations?
[ FollowRulebook rulebook parameter no_paragraph_skips ! <-- parameter of interest is no_paragraph_skips
rv ss spv;
ss = self;
if ((Protect_I7_Arrays-->0 ~= 16339) || (Protect_I7_Arrays-->1 ~= 12345)) {
print "^^*** Fatal programming error: I7 arrays corrupted ***^^";
@quit;
}
if (parameter) { self = parameter; parameter_object = parameter; }
spv = parameter_value; parameter_value = parameter;
! we won't need parameter again, so can reuse it
parameter = debugging_rules;
#ifndef MEMORY_ECONOMY;
if (debugging_rules) {
DebugRulebooks(rulebook, parameter);
process_rulebook_count = process_rulebook_count + debugging_rules;
}
#endif;
if ((rulebook >= 0) && (rulebook < NUMBER_RULEBOOKS_CREATED)) {
rv = rulebooks_array-->rulebook;
if (rv ~= EMPTY_RULEBOOK) {
if (rulebook ~= ACTION_PROCESSING_RB) MStack_CreateRBVars(rulebook);
if (say__p) RulebookParBreak(no_paragraph_skips); ! <-- parameter passed during call, set to 1 for activity rulebooks [PROBLEM LINE]
rv = rv(no_paragraph_skips); ! <-- parameter passed during call and ultimately repassed to RulebookParBreak()
if (rulebook ~= ACTION_PROCESSING_RB) MStack_DestroyRBVars(rulebook);
} else {
rv = 0;
}
} else {
if (say__p) RulebookParBreak(no_paragraph_skips); ! <-- parameter passed during call
rv = indirect(rulebook);
if (rv == 2) rv = reason_the_action_failed;
else if (rv) rv = rulebook;
}
if (rv) {
#ifndef MEMORY_ECONOMY;
if (debugging_rules) {
process_rulebook_count = process_rulebook_count - debugging_rules;
if (process_rulebook_count < 0) process_rulebook_count = 0;
spaces(2*process_rulebook_count);
if (latest_rule_result-->0 == RS_SUCCEEDS) print "[stopped: success]^";
if (latest_rule_result-->0 == RS_FAILS) print "[stopped: fail]^";
}
#endif;
} else {
if (debugging_rules)
process_rulebook_count = process_rulebook_count - debugging_rules;
latest_rule_result-->0 = RS_NEITHER;
}
debugging_rules = parameter;
self = ss; parameter_value = spv;
return rv;
];
[ RulebookParBreak no_paragraph_skips; ! <-- passed parameter
if ((no_paragraph_skips == false) && (say__pc & PARA_NORULEBOOKBREAKS == 0)) ! <-- if no_paragraph_skips == true then...
DivideParagraphPoint(); ! <-- ...never marks a new paragraph
];
The unexpected line break is generated by the line marked “[PROBLEM LINE]”. It is only invoked if there is at least one rule in the After rules, because otherwise the constant AFTER_RB will be set to the EMPTY_RULEBOOK() routine.
I think the idea here is that a paragraph break check should be made whenever moving between rulebooks, so as to process any lingering paragraph control state from previous rulebooks. As near as I can tell, in this case the unexpected line break is generated due to the leftover state from the activity rules invoked by the most recent successful action (e.g. John taking the pear).
The particular call to FollowRulebook() that makes it possible for the unexpected line break to be printed comes from ActionPrimitive() by way of BeginAction(), which is itself called as part of GenerateMultipleActions(). That last routine is what is handling the individual attempts to take specific things, as part of processing >JOHN, TAKE ALL.
Even more information...
Even more information…
It might be desirable to suppress rulebook transition paragraph breaks during processing of a multiple action. This can be done by changing the “problem line” to
if (say__p && multiflag == false) RulebookParBreak(no_paragraph_skips); ! <-- parameter passed during call [MODIFIED]
However, this alone does not fully address the issue. Unexpected line breaks will still be generated in response to some After rules. Following are some rules for study:
After waving: [will still generate unexpected line breaks, even if it's the only After rule]
say "John waves back."
After an actor taking: [will NOT by itself generate unexpected line breaks]
say "Taken by [the actor]."
To unravel this mystery, it’s necessary to look into how rulebooks are generated. Within a given rulebook, there will be a series of if... else... blocks, one for each rule placed within it. For example, if including the two After rules just mentioned it will be (with comments added):
[ B23_after
forbid_breaks ! Implied call parameter
rv ! return value
original_deadflag ! saved state
;
original_deadflag = deadflag;
! BEGIN "After waving:"
if (action == ##Wave) { ! <-- attempts to disqualify rule cheaply by looking at action name part of preamble
rv = R_767(); ! <-- invokes rule proper, where say__p will be set by response if full preamble met
if (rv) {
if (rv == 2) return reason_the_action_failed;
return R_767;
}
latest_rule_result-->0 = 0;
} else { ! <-- rule was disqualified
if (say__p) RulebookParBreak(forbid_breaks); ! <-- paragraph break check (but why here?) [SECOND PROBLEM LINE]
}
! END "After waving:"
! BEGIN "After an actor taking:"
if (action == ##Take) { ! <-- this will be true for John taking something
if (original_deadflag ~= deadflag) return 0;
if (say__p) RulebookParBreak(forbid_breaks); ! <-- paragraph break check (for 2nd and later rules only?)
rv = R_768();
if (rv) {
if (rv == 2) return reason_the_action_failed;
return R_768;
}
latest_rule_result-->0 = 0;
} else {
if (say__p) RulebookParBreak(forbid_breaks);
}
! END "After an actor taking:"
return 0; ! 2 rule(s)
];
The compiler seems to be adjusting the output depending on the the ordinal position of the rule within the rulebook. If the After waving hands: rule is left out, then the generated rulebook code will be (with comments added):
[ B23_after
forbid_breaks ! Implied call parameter
rv ! return value
;
! BEGIN "After an actor taking:"
if (action == ##Take) { ! <-- this will be true for John taking something...
rv = R_767();
if (rv) {
if (rv == 2) return reason_the_action_failed;
return R_767;
}
latest_rule_result-->0 = 0;
} else { ! <-- ...so this else block will be skipped
if (say__p) RulebookParBreak(forbid_breaks);
}
! END "After an actor taking:"
return 0; ! 1 rule(s)
];
Note that even though the remaining After rule itself modifies say__p because it prints something, the logic depends on a later call to RulebookParBreak() to actually print the paragraph break.
I do not understand the placement of the call to RulebookParBreak() within the block generated for a single-rule rulebook – where it is, it can be invoked if and only if the rule could not “apply” (i.e. the action name condition from its preamble condition wasn’t met), so whatever the code decides to do regarding paragraph breaks will necessarily be based on something that happened prior to consideration of that rule. While arguably the system is designed such that you can make paragraph checks as often as you like without causing a problem, it’s not clear to me what purpose this type of check (i.e. one done after skipping an inapplicable rule) would ever usefully serve. (The structure may be vestigial.)
The logic of the block for the second rule in the two-rule version of the rulebook makes more sense to me. Placing the paragraph break check just before invoking the rule means that the decision about whether or not to generate a paragraph break would be based on the state prior to the rule being executed, and the state of say__p and related globals will be cleared to make way for the invoked rule’s influence.
On the whole, it seems like this is an emergent behavior with multiple causes. I don’t know that there is any better solution than the suggestion above (either rileypb’s original suggestion or the slight modification shown at the top of this post) short of rebuilding the whole paragraph control system. I think there may be an extension that tries to do that, but there appears to be plenty of generated I6 that would flow from the compiler and would be unmodifiable via extension.
