PuzzleScript icon indicating copy to clipboard operation
PuzzleScript copied to clipboard

Inconsistancy in error messages related to objects that can't overlap

Open ClementSparrow opened this issue 3 years ago • 5 comments

From the unit test 'Rules with objects that "can never overlap" creates exception', made to test issue #605 :

startloop
[ Present Present ] -> [ Present Present TempCheck ] (triggers error)
[ ] -> [ ]
(more stuff)
endloop

[ Present Present ] -> [ Present Present TempCheck ] (compiles fine)
+[ ] -> [ ]
(more stuff)

With 'Present' being a property of objects on different layers.

The first rule compile fines in the rule group but not in the loop, where it triggers the error message :

 line 179 : LOTS and LOTS can never overlap, but this rule requires that to happen.

In my mind, the error message should be triggered in both situations?

ClementSparrow avatar Aug 13 '21 05:08 ClementSparrow

See also #512.

ClementSparrow avatar Aug 13 '21 05:08 ClementSparrow

Note: Even if it compiles fine, the rule in the rule group is silently discarded. This is the output of debug when the loop is commented:

Rule Assembly : (1 rules)
===========
(187) DOWN [ ] -> [ ]
===========

ClementSparrow avatar Aug 13 '21 05:08 ClementSparrow

It seems the idea was that if Present = A or B, then the rule [ Present Present ] -> [ Present Present TempCheck ] would be expanded as:

[ A A ] -> [ A A TempCheck ]
+ [ A B ] -> [ A B TempCheck ]
+ [ B A ] -> [ B A TempCheck ]
+ [ B B ] -> [ B B TempCheck ]

With the first and last of these four rules being silently discarded because they don't make sense, but the two rules in the middle being accepted? It seems to be what the code in ruleGroupDiscardOverlappingTest should be doing, but the whole rule group is then discarded.

ClementSparrow avatar Aug 13 '21 06:08 ClementSparrow

Oh this is interesting! Thanks for the report.

It's doing inference on present and present and making them the same or something like that, lol...

https://www.puzzlescript.net/play.html?p=a191f07e897cdf9a985c9db4324d4c97

Renaming the second 'present' to 'present2' (defined the same way as 'present) 'fixes' it.

In puzzlescript if you type

[ player player ]->[ ]

you get an error saying player and player can't overlap.

If you type

[ player_or_crate player_or_crate ]

Do you expect it to ever match? Trying to be consistent with the rest of puzzlescript I'd say 'no' is the correct behaviour, though with complicated rules maybe it'll be bad.

Looking at this again: [ Present Present ] -> [ Present Present TempCheck ] Do I really want to do disambiguation on an sub-cellular level? I feel like maybe I don't...

I'll have to think about this a bit more....

increpare avatar Aug 13 '21 09:08 increpare

What should

[ player_or_target player_or_target ] -> [ player_or_target ]

do?

Right now it gives an error

line 83 : TARGET and TARGET can never overlap, but this rule requires that to happen.

And I'm maybe ok with it giving a more understandable error, but maybe I should give a more useful one ("Uff. Puzzlescript's inference engine can't handle having multiple things with the same name appearing in the same cell. If you really want to do this, consider renaming them like in X [ link to example X]")

https://www.puzzlescript.net/editor.html?hack=aca5c3fc00a3e34c74120118e4d87770

increpare avatar Aug 13 '21 10:08 increpare