Fix player death event related syntaxes
Description
Fixes EffKeepInventory, EffRespawn force, ExprLevel not displaying a warning, and ExprMessage death message all not working.
Skript never had PlayerDeathEvent registered so Skript would never understand the event during parse time.
Existing syntaxes that rely on EntityDeathEvent still work as intended because PlayerDeathEvent extends EntityDeathEvent like attacker, victim etc.
This pull request also allows for
on death:
if victim is a player:
keep inventory
Target Minecraft Versions: any Requirements: none Related Issues: https://github.com/SkriptLang/Skript/issues/5657 and https://github.com/SkriptLang/Skript/issues/2285 which was closed as a Spigot bug when in reality it was Skript not working properly for the force respawn.
I'm not really a fan of putting in a hardcoded check like this into CondCompare. I would prefer us to take less invasive options, and I'll note a few possibilities below:
1. Only allow the syntax in an explicit `on death of player` declaration (this is a breaking change and may limit users more) 2. Allow the syntax in any kind of death event and just depend on the existing event instanceof check (should not be a breaking change and gives the user the most freedom, but there is less error checking for the user)The existing implementation certainly has a good balance between error checking and user freedom, but I think it's invasiveness weights it down too greatly.
The old implementation disallows usage of syntaxes as it cannot understand the condition if the expression type that goes inside the pattern is unknown. This pull request opens the ability to not silently hide errors that the user should know about and enables syntaxes to start working as Skript would then know the proper type.
There are more events that would benefit from this enhancement and not just the death event like the force respawn and others as mentioned in the first introduction message. So I don't agree with restricting to just death events.
Is this even still useful since https://github.com/SkriptLang/Skript/pull/5658 has been merged?
Is this even still useful since #5658 has been merged?
Yes, this is still valid.
Thank you for this contribution. However, after discussing with the team, we do not think that the current approach is suitable for Skript. We may consider a more general system for having syntax like CondCompare affect the ParserInstance and its data, and you are welcome to provide ideas for that. For now, though, we will be closing this PR.