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

Adding new value to existing expression breaks functionality #210

Closed
bi0qaw opened this issue Oct 16, 2016 · 11 comments
Closed

Adding new value to existing expression breaks functionality #210

bi0qaw opened this issue Oct 16, 2016 · 11 comments
Labels
enhancement Feature request, an issue about something that could be improved, or a PR improving something.

Comments

@bi0qaw
Copy link
Contributor

bi0qaw commented Oct 16, 2016

Yeah I know the title is shit...

What I did:
registered a new expression: (0¦x|1¦y|2¦z)(-| )(coord[inate]|pos[ition]|loc[ation])[s] of %vector%
executed the following script:

command /k:
    trigger:
        set {_k} to location at player
        broadcast "%x-coordinate of location at player%"
        broadcast "%x-coordinate of {_k}%"
        broadcast "%x-coordinate of player%"

Result:

140
<none>
140

The problem is that the syntax of the expression is identical to the syntax for the coordinate of a location: [the] (x|y|z)(-| )(coord[inate]|pos[ition]|loc[ation])[s] of %locations%

Not a major issue but something to be aware of.

@ZeTioZ
Copy link

ZeTioZ commented Oct 16, 2016

You can't take a location of a variable because the variable is not parsed as an entity so it will return you "" that's not an issues, you just don't understood the code you wrote (I think).

If you wrote

command /k: trigger: set {_k} to player set {_k} to {_k} parsed as player broadcast "%x-coordinate of {_k}%"

Then it will result 140 if your player is in x coordinate 140.

@bi0qaw
Copy link
Contributor Author

bi0qaw commented Oct 16, 2016

That is just wrong. The variable is a location and you can get a coordinate from a location. If I don't register the first syntax it works perfectly fine.

@TheBentoBox
Copy link
Member

TheBentoBox commented Oct 16, 2016

That doesn't make sense @ZeTioZ , why would the variable need to be "parsed as an entity" (which is impossible, by the way) to get a coordinate from it? You can actually only get coordinates from locations; whenever you do x-coordinate of player, the only reason it works is because Skript knows you want the x-coordinate of the player's LOCATION and does it for you automatically. An entity on its own as a concept doesn't have a coordinate, it's all stored in the location of it.

Also:

set {_k} to player
set {_k} to {_k} parsed as player

Setting something to a type then setting it to itself parsed as that type again is so unnecessary and redundant. I get doing that if the return type is ambiguous or you're parsing into a string to make definite comparisons, but here those two lines of code just look silly. {_k} is clearly and unambiguously a player object here, no need to parse it again.

@ZeTioZ
Copy link

ZeTioZ commented Oct 16, 2016

I parsed that because if you try to do "if {_k} is online:" it will return you {_k} is not a player or something like that, because I think it look the variable as a String and not as a player it's just for that ^^, and for the location, yes I did a mistake so sorry about that ^^

@bensku
Copy link
Member

bensku commented Oct 17, 2016

Skript is ridiculous when parsing user-defined variables as part of expressions. It doesn't remember original type of variable; instead it tries to convert the variable to whatever type one of these expressions could take. In this case, Skript is converting location to vector, which naturally doesn't work.

Not tagging this help wanted, since there is probably no one experienced enough with Skript around.

Last time I tried to fix this, it didn't go very well. It broke everything, so after few days of messing I gave up. I'm not particularly interested to try again...

@bensku bensku added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Oct 17, 2016
@bi0qaw
Copy link
Contributor Author

bi0qaw commented Oct 23, 2016

Just wanted to add that the same thing happens if you have an expression that allows two different types. For example: %number/vector% * %number/vector%. If you pass a variable containing a vector into this expression it will change it's type to number.

@bensku
Copy link
Member

bensku commented Oct 24, 2016

Whelp, this is bad... Very bad.

I wonder if this issue disappears with parser rework :) We'll see it (hopefully) soon...

@bensku
Copy link
Member

bensku commented May 1, 2017

This might actually be solved since dev26. Took ages, sadly...

@bensku
Copy link
Member

bensku commented Jul 28, 2017

Long story short, it was not fully fixed. I have identified to root cause of problem in Skript parser, though. When parsing multiple potential types, they're separately until creating converted expression to some of them succeeds. Too bad that for variables, it always succeeds but results in an expression which does not contain any useful data.

@Snow-Pyon
Copy link

If my memory servers me right, this was fixed in one of the latest updates so I think this can be closed now.

@bensku
Copy link
Member

bensku commented Sep 11, 2017

I'm not sure if anyone actually tested the fix, beyond my testing without any addons...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request, an issue about something that could be improved, or a PR improving something.
Projects
None yet
Development

No branches or pull requests

5 participants