Assigning variable type corrupts getter

[Whitealion]

Tested versions

Reproducible in v4.2.1.stable.mono.official [b09f793]

System information

Godot v4.2.1.stable.mono - Windows 10.0.19045 - Vulkan (Forward+) - dedicated AMD Radeon RX 6750 XT (Advanced Micro Devices, Inc.; 31.0.24027.1012) - AMD Ryzen 7 2700X Eight-Core Processor (16 Threads)

Issue description

Setting type of variables that keep track of current Health, Energy and Shield impacts getters of other variables associated with each tracker.
Getting one variable works as intended, but getting at least two results in corruption.
Issue happens outside of setter as well.
Example code shows Health variables and their setters and getters and how they operate together.

Example videos:
Taking Damage
Gaining Shield

@export var maxHealth : int = 40
var realMaxHealth : int : get = _getMaxHealth
func _getMaxHealth():
	return (maxHealth + maxHealthAdd) * maxHealthMult
var maxHealthAdd : int = 0 :
	set(val):
		maxHealthAdd = val
		health_max_changed.emit(realMaxHealth)
var maxHealthMult : float = 1.0 :
	set(val):
		maxHealthMult = val
		health_max_changed.emit(realMaxHealth)
#########################################################################################################
var currentHealth : int = realMaxHealth : # the ': int' here causes corruption
#########################################################################################################
	set(val):
		var a = currentHealth
		var b = realRegenValueHealth
		var c = a + b
		var d = currentHealth + realRegenValueHealth # only corrupted value, example in pictures below with breakpoint set here
		currentHealth = clamp(val, 0, realMaxHealth)
		health_changed.emit(currentHealth)
		if currentHealth == 0:
			states.clear()
			states.append(MyGlobals.States.Dead)
@export var regenRateHealth : float = 1
var realRegenRateHealth : float : get = _getRegenRateHealth
func _getRegenRateHealth():
	return (regenRateHealth + regenRateHealthAdd) * regenRateHealthMult
var regenRateHealthAdd : float = 0 :
	set(val):
		regenRateHealthAdd = val
		health_regenRate_changed.emit()
var regenRateHealthMult : float = 1.0 :
	set(val):
		regenRateHealthMult = val
		health_regenRate_changed.emit()
@export var regenValueHealth : int = 1
var realRegenValueHealth : int : get = _getRegenValueHealth
func _getRegenValueHealth():
	return (regenValueHealth + regenValueHealthAdd) * regenValueHealthMult
var regenValueHealthAdd : int = 0 :
	set(val):
		regenValueHealthAdd = val
		health_regenValue_changed.emit()
var regenValueHealthMult : float = 1.0 :
	set(val):
		regenValueHealthMult = val
		health_regenValue_changed.emit()
var regenTimerHealth : float = 0.0

Regening Health

Taking Damage

Steps to reproduce

Wait for either the regen function to be called or walk into an attack hitbox to see the Health values corrupting
Walk into circle area to see Shield values corrupting
Wait for energy regen timer to see its values corrupting

*edit: Press ENTER when loading into main scene to load the debug scene

All necessary code is in res://scenes/characters/baseChara/baseChara.gd

Minimal reproduction project (MRP)

Pruned project I was working on when I discovered this: CorruptionExample.zip

[huwpascoe]

Can't test right now but it might be important:
Everything is strongly typed except for the functions.

The syntax is

func _getMaxHealth() -> int:

[kleonc]

MRP simplified:

@tool
extends EditorScript

var int_prop: int: get = float_returner # Warning: Line 4 (NARROWING_CONVERSION): Narrowing conversion (float is converted to int and loses precision).

func float_returner():
	return 1.0

func _run() -> void:
	var not_really_an_int: int = int_prop
	print(type_string(typeof(not_really_an_int)))
	print(not_really_an_int)
	print(not_really_an_int + 1)
	print(not_really_an_int + 1.0)

Output:

float
1
4607182418800017409
4607182418800017400

Confirmed in:

• v4.0.4.stable.official [fc0b241]
• v4.1.4.stable.official [fe0e8e5]
• v4.2.2.stable.official [15073af]
• v4.3.dev6.official [89850d5]
• v4.3.dev.custom_build [17a8126]

[Whitealion]

@huwpascoe
Doing that results in this when regening:

Expected regen should be 40+1, but the passed val is already way overblown, and then var d somehow turned into 1 when doing the same math that activated this setter.

While when taking damage it seems to do currentHealth - damage (40 - 10) fine, but the regen math (var d) is still corrupted and results in 1.

Code snippet of debug breakpoint:

var currentHealth : int = realMaxHealth :
	set(val): # val is what current health should be set to after any damage/heal math has been done and is then clamped
		var a = currentHealth
		var b = realRegenValueHealth
		var c = a + b
		var d = currentHealth + realRegenValueHealth # still the only one corrupted
		var e = realMaxHealth
		currentHealth = clampi(val, 0, realMaxHealth) # even did clampi for good measure, didn't help

[pirey0]

Made some progress on this issue:

For the following code:

@tool
extends EditorScript

var int_prop: int: get = float_returner # Warning: Line 4 (NARROWING_CONVERSION):

func float_returner():
	return 1.0

func _run() -> void:
	var not_really_an_int: int = int_prop
	print(not_really_an_int + 1.0)

the gdscript_vm runs:

call: float_returner
    return 1.0 (FLOAT)
assign: not_really_an_int 1.0 (FLOAT)  #⚠️ should be assign_typed_builtin so that the conversion can happen
operator_validated: + not_really_an_int 1.0 #⚠️ runs OpeatorEvaluatorAdd<double, __int64, double> but 1.0 (FLOAT) is passed and reinterpreted as __int64
call_utility: print

why is the VM using assign and not assign_typed_builtin?

• when GDScriptByteCodeGenerator::write_assign is called for var not_really_an_int: int = int_prop the type of int_prop is set to INT (⚠️). since this matches the target variable type the compiler sees no reason to use "assign_typed_builtin" instead of the faster "assign" op

why is the type of int_prop set to INT?

• when the gdscript_compiler processes the getter call, the type of the member variable is used instead of the return type of the getter. ⚠️
  (This is a reasonable assumption for all cases, except for this specific one, in which a getter returns a different type with an implicit conversion)
  https://github.com/godotengine/godot/blob/daa81bbb7d1c6d75d1711595604178ee62a5801d/modules/gdscript/gdscript_compiler.cpp#L304C9-L308C21
  (Update: While this is technically a mistake, the bug still happens even if we remove the type specifier here..)

why is the VM using assign and not assign_typed_builtin? (part 2)

• an alternative issue could be that the use_conversion_assign flag is not set by the analyzer

why is the use_conversion_assign flat not set?

• because GDScriptAnalyzer::resolve_assignable again assumes the type of the property to be int ⚠️
  (reduce_identifier_from_base again reads the type of the member variable and not the return type of the getter)

How I think we could fix this:

1. gdscript_compiler/analyzer should use the return type from the getter and not the type of the member
2. we add a casting step to ensure the getter value of the expected type
3. we don't allow getters to return different types also when an implicit conversions is possible