Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

on damage: message "something" does not error #1415

Closed
Nicofisi opened this issue Jul 26, 2018 · 11 comments
Closed

on damage: message "something" does not error #1415

Nicofisi opened this issue Jul 26, 2018 · 11 comments
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. completed The issue has been fully resolved and the change will be in the next Skript update. priority: low Issues that are not harmful to the experience but are related to useful changes or additions.

Comments

@Nicofisi
Copy link
Member

Code

on damage:
    message "damage"

this code loads with no errors, yet does nothing at runtime (neither sends the message to the victim nor to the attacker)

Expected behavior

It should error, saying that in the damage event either attacker or victim should be explicitly specified

@Pikachu920
Copy link
Member

to add some info to this: it seems like the error is going off but gets stuck in a log handler somewhere

@BaeFell
Copy link
Member

BaeFell commented Jul 28, 2018

I always thought Skript errored saying that you need to specify the player receiving the message? Such as the attacker or the victim.

@Nicofisi
Copy link
Member Author

@nfell2009 iirc people on skUnity said that it only errors when you explicitly use event-player or event-entity, aka not here

@Ofus-Will
Copy link

I thought it defaulted to the victim? 🤔

@Nicofisi
Copy link
Member Author

@OfusMC nope, it does completely nothing

@TheBentoBox TheBentoBox added bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. priority: low Issues that are not harmful to the experience but are related to useful changes or additions. labels Jul 29, 2018
@TheBentoBox
Copy link
Member

Not really sure what prio to put this at, I'm considering medium since I know many have encountered this behavior before including myself and it's unclear what's happening.

@TheLimeGlass
Copy link
Collaborator

Andrew made a fix for this iirc

@28andrew
Copy link
Contributor

28andrew commented Jul 31, 2018

I tried to but I couldn't figure out how to print the error message in the part of the SkriptParser where I wrote the patch (Github name is set wrong :|): master...xXAndrew28Xx:fix-1415 (Skript.error would not work in this location for some reason)

@TheLimeGlass
Copy link
Collaborator

Isn't it the same system like the parse expression? Call it to listen then parse and an error gets printed

@28andrew
Copy link
Contributor

Hmm I'll look into that. Thanks.

TheLimeGlass added a commit to TheLimeGlass/Skript that referenced this issue Feb 23, 2020
@TheLimeGlass
Copy link
Collaborator

TheLimeGlass commented Feb 23, 2020

Found and fixed bug;

The issue is this mess https://github.com/SkriptLang/Skript/blob/master/src/main/java/ch/njol/skript/registrations/EventValues.java#L165-L181

Readable version:

for (EventValueInfo<?, ?> ev : eventValues) {
    if (!c.isAssignableFrom(ev.c)) // Projectile would pass
        continue;
    if (!EventValues.checkExcludes(ev, e)) // EntityDamageByDamage (ev.event) is not assignable from EntityDamageEvent
        return null;
    if (ev.event.isAssignableFrom(e)) // EntityDamageByDamage (ev.event) is not assignable from EntityDamageEvent
        return (Getter<? extends T, ? super E>) ev.getter;
    if (!e.isAssignableFrom(ev.event)) // EntityDamageByDamage (ev.event) is not assignable from EntityDamageEvent
        continue;
    return new Getter<T, E>() {
        @Override
        @Nullable
        public T get(E event) {
            // This looks like a fix that was added afterwards, because lots of related events can get here.
            if (!ev.event.isInstance(event)) // EntityDamageByDamage (ev.event) is not an instance of EntityDamageEvent
                return null; // Why EffMessage with no command sender doesn't work for default expressions.
            return ((Getter<? extends T, E>) ev.getter).get(event);
        }
    };
}

The issue is that the event value https://github.com/SkriptLang/Skript/blob/master/src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java#L402 is taking priority, the projectile is an entity/commandsender, so it passes those isAssignableFrom, it passes all the event isAssignableFrom because EntityDamageByEntity is not assignable from EntityDamageEvent it's vice versa, and the getter will return null because EntityDamageByEntity is not an instance of EntityDamageEvent, so the EffMessage default expression sends the message to no consolesender when attempting to grab a default event value.

@FranKusmiruk FranKusmiruk added the completed The issue has been fully resolved and the change will be in the next Skript update. label Jul 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. completed The issue has been fully resolved and the change will be in the next Skript update. priority: low Issues that are not harmful to the experience but are related to useful changes or additions.
Projects
None yet
Development

No branches or pull requests

9 participants