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

GDScript line incorrectly marked as unsafe #88152

Open
Vivalzar opened this issue Feb 9, 2024 · 7 comments
Open

GDScript line incorrectly marked as unsafe #88152

Vivalzar opened this issue Feb 9, 2024 · 7 comments

Comments

@Vivalzar
Copy link

Vivalzar commented Feb 9, 2024

Tested versions

v4.3.dev3.official [36e943b]

System information

Godot v4.3.dev3 - Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3060 Ti (NVIDIA; 31.0.15.5123) - AMD Ryzen 5 5600X 6-Core Processor (12 Threads)

Issue description

The line 13 below is marked as unsafe while the line 12 is marked as safe:
image

Both lines are marked as safe if I replace the line 3 by:

const EXIT_DIR : Array[Vector2i] = [

It should work the same with both syntax.

Steps to reproduce

Create a new GDScript and paste this code:

extends Node

const EXIT_DIR := [
		Vector2i( 0, -1),
		Vector2i( 1,  0),
		Vector2i( 0,  1),
		Vector2i(-1,  0)]

var room := Vector2i(10, 10)

func change_room(i: int):
	room += EXIT_DIR[0]
	room += EXIT_DIR[i]

Minimal reproduction project (MRP)

See above.

@AThousandShips
Copy link
Member

AThousandShips commented Feb 9, 2024

Those two pieces of code aren't equivalent though, the types are different, the safety analysis might be missing some cases though but the change to the code means different types

This:

const EXIT_DIR := [

Which is equivalent to:

const EXIT_DIR : Array = [

Is not equivalent to:

const EXIT_DIR : Array[Vector2i] = [

@Mickeon
Copy link
Contributor

Mickeon commented Feb 9, 2024

I have noticed this issue often. Still somewhat of a nuisance, especially since these arrays cannot really be modified.

@AThousandShips
Copy link
Member

We should be able to do static checking on the elements of the array, but the clarification is that the issue isn't the type of the array but that the safety check doesn't seem to go deeper than the array type return, this might not be entirely simple either I don't know the specifics here

@Vivalzar
Copy link
Author

Vivalzar commented Feb 9, 2024

As per #46830, I thought the full array type was infered...
In any case, it could be a static analysis issue or an infered type issue.

@AThousandShips
Copy link
Member

The type is inferred, the type being an untyped array, otherwise you'd have bugs and unforseen behavior, like:

var arr := [1, 2, 3]
arr.push_back("Test")

That should still work and not throw an error

The issue is that the unsafe check doesn't look into the array, the type shouldn't be inferred to Array[int] here IMO

@Cammymoop
Copy link
Contributor

Note that even an array declared like const arr := [1, 2, 3] can't be safely inferred as Array[int] because the array itself is not constant and can be modified just as in @AThousandShips's example

@HolonProduction
Copy link
Member

Note that even an array declared like const arr := [1, 2, 3] can't be safely inferred as Array[int] because the array itself is not constant and can be modified just as in @AThousandShips's example

That's not the case for recent Godot versions. If you call e.g. append you will get an error like array is in read only mode. Same for dictionaries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants