Skript: Erroneous local variable type warnings

[jaylawl]

... well, i'm beat to be honest:
https://snag.gy/I0pVx7.jpg
https://snag.gy/3sgMBl.jpg

I can't scroll back far enough to read all these messages. Everything was working just fine before upgrading, no errors. Couldn't find this change in the changelog - what's going on?

[Nicofisi]

Please, send the full script or anything that can provide more context - where are these variables set/changed/used?

[jaylawl]

        if hasTagValue(victim, "npc-ball") = true:
            set {_owner} to getTagValue(victim, "npc-ball")
            loop all entities in world "golf":
                loop-entity's uuid = {_owner}
                set {_owner} to loop-entity
                exit loop
            distance between {_owner} and victim < 3

It's complaining about this for example local variable '_owner' is entity, not location, referring to the last line of the above

[TheBentoBox]

Setting this to high prio since it seems pretty problematic.

@bensku I believe the issues people are experiencing are due to the new local variable type check errors not supporting default converters. Not sure if that's really fixable (especially when it comes to converters from addons) but if not then I think the functionality needs to be removed since it's giving errors where it simply shouldn't (like reported here, blocks should be perfectly acceptable here).

[Nicofisi]

I'd say that it's something pretty critical and it'd be perfect if a new release with only a fix was released within the next few days

[Pikachu920]

i really don't think variable type checking is a good idea

[Nicofisi]

Actually it's only a warning, I don't think this prevents scripts from loading - https://github.com/SkriptLang/Skript/blob/4edc5b09fae20b7aea2147a5432b27a00aea7f59/src/main/java/ch/njol/skript/lang/Variable.java#L176-L178

If it really doesn't then the issue probably isn't that critical, but in reality I think it still is

[jaylawl]

It did break things for me

[TheLimeGlass]

The Type hints don't check converters/changers so everything that does will error.

[Pikachu920]

imo, type hints should be disabled for now until they are smarter (e.g. recognizing a variable as having multiple potential types and respecting converters and object expressions being assumed as correct). Even then it should still be an option really as it's becoming increasingly common for addons to manipulate variables in ways skript can't predict. i wouldn't be opposed to removing them completely, either. skript really isn't a typed language.

[fl-y]

https://pastebin.com/kT7Ry9i7
Used to have 0 errors
After update, 356 warnings, and most stuff broken.
Currently unusable.

[bensku]

Note that type hints produce warnings, not errors. If scripts are failing, it is likely that something else is broken.

[jaylawl]

Makes sense. I was also having errors, but i have no way of reading them because they're spammed away by the warnings. Unfortunately my server's console only shows a limited portion of the latest log.

[bensku]

Need a test case without addons. I wrote a simple one myself, but it does not trigger the bug in release build, so I have no idea if my fix is working.

[bensku]

Ok, I fixed some stuff, but due to limited test material I had, not sure that everything is ok now.

• Conditional blocks no longer produce inaccurate type hints
• Converters are used if no direct match is found

[jaylawl]

Significantly less, but found some more with dev37b (marked with ####):
https://snag.gy/nzPYcv.jpg

every second:

    loop all zombies in world "golf":
        delete {_npc}, {_ball}, {_hole}, {_vector}, {_destination} and {_potential-balls::*}
        hasTag(loop-entity, "jnpc-behaviour:putting") = true
        set {_npc} to loop-entity

        loop all chickens in world "golf":
            hasTag(loop-entity-2, "ball-type:putting-practice") = true
            if hasTag(loop-entity-2, "npc-ball:%{_npc}'s uuid%") = true:
                set {_ball} to loop-entity-2
                exit loop
            else:
                distance between {_npc} and loop-entity-2 < 16 ######################
                hasTagValue(loop-entity-2, "npc-ball") = false
                add loop-entity-2 to {_potential-balls::*}

        if {_ball} is not set:
            {_potential-balls::*} is set
            set {_ball} to random element out of {_potential-balls::*}
            addTag({_ball}, "npc-ball:%{_npc}'s uuid%")
            jnpc_findPuttingHole({_npc}, {_ball})

        if {_ball} is set:
            (x of {_ball}'s velocity + z of {_ball}'s velocity) = 0
            if getTagValue({_npc}, "npc-hole-x") is not set:
                jnpc_findPuttingHole({_npc}, {_ball})
            getTagValue({_npc}, "npc-hole-x") is set
            set {_x} to getTagValue({_npc}, "npc-hole-x")
            set {_z} to getTagValue({_npc}, "npc-hole-z")
            set {_hole} to location(("%{_x}%" parsed as number), y-coord of {_ball}'s location, ("%{_z}%" parsed as number), {_ball}'s world)

        set {_last-action} to getTagValue({_npc}, "last-action")
        if {_last-action} is not set:
            set {_last-action} to "unknown"
        if {_ball} and {_hole} are set:
            if {_last-action} = "prepare_stroke":
                setTagValue({_npc}, "last-action", "stroke")
                distance between {_npc} and {_ball} < 3 ######################
                (x of {_npc}'s velocity + z of {_npc}'s velocity) = 0
                set {_npc}'s target to {_ball}
         [...]

/edit:
i have added the condition {_npc} is set, but it does not make a difference
changing the lines to distance between {_npc}'s location and {_ball} < 3 does not help either

/edit2:
More: https://snag.gy/aGN5e2.jpg

on dismount:
    event-entity's vehicle's name = {@bench-id}
    set {_entity} to event-entity's vehicle
    set {_loc} to {_entity}'s location
    set y-coord of {_loc} to 1
    wait 1 tick
    teleport {_entity} to {_loc}
    set {_entity}'s health to 0

[TheBentoBox]

Looks like it's doing type checking even for unset variables, which it shouldn't do.

[jaylawl]

Apart from those warnings however, everything seems to be working fine. The only notable thing that's broken now is TusKe, which is a shame because my gut feeling says TukeNuke won't work on it anymore. But that's another story. tl;dr: seems stable

[TheBentoBox]

That's great to hear, thanks for the report. We'll be working through those false warnings (and if it seems to hard to resolve them all, bensku will just remove that new feature).

I will message Tuke personally but I'm not sure that will help anything. Can't hurt though.

[SlimeDog]

Here are a few more. No warnings prior to dev37. dev37b reduced the number of warnings, but these still exist. Note that scripts appear to work correctly, despite type warnings.

[18:18:04] [Server thread/WARN]: [Skript] Local variable '_player' is text, not inventory holder (equip-luckyblock.sk, line 44: loop all items in inventory of {_player}:')
[18:18:04] [Server thread/WARN]: [Skript] Local variable '_player' is text, not inventory holder (equip-luckyblock.sk, line 52: remove the loop-item from the inventory of {_player}')
[18:18:05] [Server thread/WARN]: [Skript] Local variable '_TARGET_WORLD' is text, not world (start-acid-rain.sk, line 45: set {_TARGET_WEATHER} to weather in {_TARGET_WORLD}')

equip-luckyblock.sk

command /equip-luckyblock <text=none>:
    description: give player luckyblocks
    usage: /equip-luckyblock player
    permission: command.equip
    trigger:
        set {_DEBUG} to true
        set {_DEBUG} to false

        set {_player} to arg 1
        if {_player} is "none":
            set {_player} to the player

        # Make sure invoker (and player?) is in the correct world
        if world is not "world_luckyblock":
            exit

        # Remove existing luckyblocks
        # loop all items in player's inventory:
        loop all items in inventory of {_player}: ########## line 44
            if name of the loop-item contains "Lucky Block":
                # remove the loop-item from the inventory of the player
                remove the loop-item from the inventory of {_player} ########## line 52

start-acid-rain.sk

on weather change:
    set {_DEBUG} to true
    set {_DEBUG} to false

    # Set the target world, the world in which rain needs to be started by this skript.
    set {_TARGET_WORLD} to "world_yellowdog"

    # Set minimum and maximum storm duration, in seconds.
    set {_MIN_STORM_DURATION} to 120
    set {_MAX_STORM_DURATION} to 600

    # Set the random threshold: something less between 0 and 9.
    # Values less than _RANDOM_THRESHOLD will start rain in _TARGET_WORLD
    set {_RANDOM_THRESHOLD} to 4

    # If the weather phase didn't actually change, don't go further.
    new weather is not equal to old weather

    # Verify that the new weather phase is rain or thunder.
    set {_NEW_WEATHER} to new weather
    "%{_NEW_WEATHER}%" is "rain" or "thunder"

    # Verify that the rainy world is not the target world.
    event-world is not {_TARGET_WORLD}

    # If it's already raining in target world, we needn't go further.
    set {_TARGET_WEATHER} to weather in {_TARGET_WORLD} ########## line 45

[TheBentoBox]

The scripts are meant to work correctly regardless; that's why they're warnings, not errors. But thank you for the extra info. That one with the world variable is weird in particular since string -> world is a converter which should be covered by the recent fix.

[fl-y]

Tried out the bugfix version.
Doesn't spam the console at least... but a lot of things that used to work with the previous version simply don't work.
I understand that it's common courtesy to specify exactly which part of my skripts are not working, but I currently use more than 500kb of skript and thus simply don't have the time to find and pinpoint exactly where.
The weird thing is that skript isn't throwing a single error but simply doesn't work with most mechanics that I created. Warnings still spam when I reload skript though(pretty much the same amount as before)

[TheBentoBox]

but I currently use more than 500kb of skript and thus simply don't have the time to find and pinpoint exactly where.

At least give the error messages, my guy. If you're going to provided quite literally nothing then the comment is useless. I'm not trying to be rude, but we really can't do anything with the amount of info you've provided (i.e. zero, "there are errors" is not valuable info).

If you want things fixed, you can't just say you "don't have time" to properly report errors - you'll need to find the time. If there are no actual error messages, we need to know what code is broken.

[TheBentoBox]

@jaylawl and @SlimeDog how's dev37c perform for you? Bensku released it this morning with even more type hint fixes. It should address most everything now.

[SlimeDog]

Just returned home, loaded up dev37c, and tested. Looks good. All the warnings on my scripts are gone. Thanks.

[jaylawl]

Looks good here aswell!

[bensku]

Finally!

[jaylawl]

https://vignette.wikia.nocookie.net/icarly/images/5/58/But_wait_theres_more.jpg/revision/latest?cb=20121018075250

Error: https://snag.gy/3tPlMG.jpg
Related code: (line ###)

        hasTagValue(victim, "npc-ball") = true:
            set {_owner} to getTagValue(victim, "npc-ball")
            loop all entities in world "golf":
                loop-entity's uuid = {_owner}
                set {_owner} to loop-entity
                exit loop
            distance between {_owner} and victim < 3 ###################
            send "%colored {_owner}'s name%&r: Hey, my ball!" to attacker

[jaylawl]

More of these errors: https://snag.gy/UXiHoW.jpg
Code: https://hastebin.com/foxetiwigi.makefile

/edit: maybe this should be re-opened

[Pikachu920]

are you sure this isn't related to #1394?

[jaylawl]

to be honest i don't know which it is...

[Pikachu920]

is it in a function? I'll reopen for now until @bensku responds

[jaylawl]

Yeah, both of which i posted are within functions

[Pikachu920]

ok, it's likely related to #1394 then but i will still let bensku take a look at this before it's closed for good

[bensku]

Related to #1394