Skript icon indicating copy to clipboard operation
Skript copied to clipboard

Damage Cause Literals unreliable when used in OR list in condition

Open sovdeeth opened this issue 3 years ago • 2 comments

Skript/Server Version

[18:30:25 INFO]: [Skript] Skript's aliases can be found here: https://github.com/SkriptLang/skript-aliases
[18:30:25 INFO]: [Skript] Skript's documentation can be found here: https://skriptlang.github.io/Skript
[18:30:25 INFO]: [Skript] Skript's tutorials can be found here: https://docs.skriptlang.org/tutorials
[18:30:25 INFO]: [Skript] Server Version: git-Paper-17 (MC: 1.19)
[18:30:25 INFO]: [Skript] Skript Version: 2.6.3
[18:30:25 INFO]: [Skript] Installed Skript Addons: None
[18:30:25 INFO]: [Skript] Installed dependencies: None

Bug Description

When using single damage cause literals in a conditional, like damage cause is poison, they work fine. However, when used in combination, eg: damage cause is poison or unknown, they can behave erratically. Some work fine in combination, like fall or suffocation, while others will either simply not work or cause parsing errors.

Including unknown in a list occasionally seems to trigger the following parsing error: image but if fire is also included (among other combinations), the error disappears. (eg: fall or unknown or suffocation or fire).

Others, like fire or wither, sometimes seem to work and sometimes don't, depending on what else is in the list with them. I've only found unknown to throw parsing errors, though. I couldn't find a reliable pattern overall, but unknown seems to be a bad one, and suffocation seems to be reliably good.

I also did not test all damage causes, much less all combinations. I experimented with different combinations and found plenty of issues within the range I tested in, though.

Expected Behavior

These damage cause literals should work without error, and work consistently. Damage causes in the list in the test code should broadcast in list, not not in list.

Steps to Reproduce

I tested by being in survival mode, using the following code, and running /test-damage.

on damage:
    broadcast "damage: %damage cause%"
    # change this line to test combinations
    damage cause is lava or fire or burning or wither or poison or unknown or suffocation:  
        broadcast "in list"
    else:
        broadcast "&cnot in list"
    cancel event

command /test-damage:
    trigger:
        # lava 
        set block at player to lava
        wait 1 second
        # fire
        set block at player to fire
        wait 1 seconds
        # burning
        set block at player to air
        wait 1 second
        extinguish player
        # wither
        apply wither 3 to player for 1 seconds
        wait 1 second
        # poison
        apply poison 3 to player for 1 seconds
        wait 1 second
        # unknown
        damage player by 1 heart
        wait 1 second
        # suffocation
        set (blocks within (player ~ vector(1.5,2,1.5)) and (player ~ vector(-1.5,0,-1.5))) to stone
        wait 5 ticks
        set (blocks within (player ~ vector(1.5,2,1.5)) and (player ~ vector(-1.5,0,-1.5))) to air

Errors or Screenshots

image

Example partial output of /test-damage: (list used: lava or fire or burning or wither or poison or unknown or suffocation) image

Other

The damage causes all behave correctly when parsed explicitly as damage causes: "wither" parsed as damage casuse.

Agreement

  • [X] I have read the guidelines above and affirm I am following them with this report.

sovdeeth avatar Jul 28 '22 01:07 sovdeeth

If I had to guess the problem is that fire and lava are also block-related words and wither and poison are potion-related words, so it's probably checking whether the fire damage cause is the fire block material, which it isn't, and so correctly telling you it isn't in the list.

This is a really difficult problem to fix reliably: we don't have great insights into what the user might be looking for when they say fire and the way of making it explicit (parsing it) is cumbersome and unhelpful.

A possible way to improve this for similar cases might be to have the comparison store some insight into what it's looking to check, so is knows it's looking for a damage cause, and we could maybe prioritise this when parsing the stuff on the other side, but it'll be quite complex.

Moderocky avatar Aug 03 '22 11:08 Moderocky

If I had to guess the problem is that fire and lava are also block-related words and wither and poison are potion-related words, so it's probably checking whether the fire damage cause is the fire block material, which it isn't, and so correctly telling you it isn't in the list.

This is a really difficult problem to fix reliably: we don't have great insights into what the user might be looking for when they say fire and the way of making it explicit (parsing it) is cumbersome and unhelpful.

A possible way to improve this for similar cases might be to have the comparison store some insight into what it's looking to check, so is knows it's looking for a damage cause, and we could maybe prioritise this when parsing the stuff on the other side, but it'll be quite complex.

%damage cause% should return a damage cause related fire. We know that at parse time. It shouldn't be as difficult as you think. This is a bug that keeps coming up, but gets fixed some way, and then it comes back. This currently doesn't happen for event-values as I fixed the order in which they read. I've written this somewhere that expressions needs to do this next somewhere in the issues.

If the expression is %object%, then yes. It would be hard to tell the difference between a damage cause and an itemtype at parse time.

TheLimeGlass avatar Aug 03 '22 12:08 TheLimeGlass

Fixed in 2.7.0-beta3

TheLimeGlass avatar Jul 11 '23 17:07 TheLimeGlass