curiousdannii-testing / inform7-imported-bugs

0 stars 0 forks source link

[I7-1958] [Mantis 1994] Adjacency test for locations behaves unexpectedly for off-stage things #126

Closed curiousdannii-testing closed 2 years ago

curiousdannii-testing commented 2 years ago

Reported by : otistdog

Description :

The phrase "location of " can evaluate to nothing for off-stage things. The condition "X is adjacent to Y" involves an I6 function, TestAdjacency(), that does not guard against either X or Y being nothing.

This can result in either a confusing "true" result for the adjacency test (in cases where X is off-stage) or a run-time error (in cases where Y is off-stage).

It's probably better to avoid this issue by using a different condition (e.g. "when the dog is in an adjacent room") but the underlying issue seems like it might be worth addressing.

Steps to reproduce :

"Bug Report"

Woods is a room.

Back 40 is a room. It is east of Woods.

Corn Field is a room. It is east of Back 40. South from Corn Field is nothing.

Yard is a room. It is east of Corn Field. North from Yard is Corn Field. West from Yard is nothing.

An animal called a dog is in Woods.

[After going when (the location of the dog) is adjacent to (the location of the player): [this version causes unexpected behavior]
    say "[The dog] is barking nearby...";
    continue the action.]

After going when (the location of the player) is adjacent to (the location of the dog): [this version causes run-time errors!]
    say "[The dog] is barking nearby...";
    continue the action.

Instead of attacking the dog:
    now the dog is off-stage;
    say "The dog runs off before you can even get close."

test me with "e / e / e / n / w / w / hit dog / e / e / e / n"

Additional information :

[Note: These details are provided from a Windows 6L38 installation, but the behavior is confirmed within Linux 6M62. I don't think the underlying cause is any different.]

The "After going..." rule creates a function with preamble:

! After going when the dog is not following and ( the location of the player ) is adjacent to ( the location of the dog ):
[ R_777 ;
if ((((action ==##Go) && (actor==player) && (self=actor,true) && ((((~~(((Adj_82_t1_v10(I128_dog))))))) && (((TestAdjacency(LocationOf(I128_dog),LocationOf(player))))))))) { ! Runs only when pattern matches

As you can see, the order of X and Y are submitted as parameters to TestAdjacency() in reverse order from their use in the I7 condition. The function called is:

! ==== ==== ==== ==== ==== ==== ==== ==== ==== ====
! WorldModel.i6t: Adjacency Relation
! ==== ==== ==== ==== ==== ==== ==== ==== ==== ====

[ TestAdjacency R1 R2 i row;
row = (R1.IK1_Count)*No_Directions;
for (i=0: i, row+)
if (Map_Storage-->row == R2) rtrue;
rfalse;
];

The run-time error seems to come from an attempt to access the IK1_Count parameter of the first parameter, in this case 0 for the nothing that is the correct "location" of the dog.

When parameters in the condition are reversed, the location of the player is the R1 parameter, and this evaluates to the location as expected. However, the data present in Map_Storage contains zeroes for all directions that go "nowhere", so the test returns true if the R2 parameter is nothing and at least one direction in the R1 location is not mapped to another actual location.

I would suggest a straightforward change to TestAdjacency() such that it returns false if either R1 or R2 is nothing. If, for design reasons, it is desirable that two off-stage things be considered adjacent, that's also easy enough to handle.

imported from: [Mantis 1994] Adjacency test for locations behaves unexpectedly for off-stage things
  • status: Closed
  • resolution: Resolved
  • resolved: 2022-04-11T00:48:13+10:00
  • imported: 2022/01/10
curiousdannii-testing commented 2 years ago

557058:4c095ffd-6d6f-47ce-9e73-77c613347b86:

Comment by zarf :
> I would suggest a straightforward change to TestAdjacency() such that it returns false if either R1 or R2 is nothing.

I agree that would produce the right results. (And offstage things should never be adjacent to anything.)

The additional checks could be placed in TestAdjacency() or in the generated code that invokes TestAdjacency().

curiousdannii-testing commented 2 years ago

61eedb62875fc10070240916:

Fixed via this commit: https://github.com/ganelson/inform/commit/2a5197c424bd215981006a5befe2de57c092a311

Comment by Graham Nelson:
Accepting the recommendation to put a test into TestAdjacency(R1, R2) requiring both R1 and R2 to exist: that was clearly always an oversight.