Add prop arrays + tests

[tudddorrr]

No description provided.

[claude]

---

PR Review: Add prop arrays + tests

🔴 Bug in remove_from_prop_array

In entity_with_props.gd, when value_exists is false, the code unconditionally pushes a null sentinel even if the array still has real values:

if !value_exists:
    props.push_back(TaloProp.new(array_key, null))  // wrong: corrupts active array
    push_error(...)
    return

Scenario: array items[] contains ["sword", "shield"], caller invokes remove_from_prop_array("items", "potion"). After the call, props contains [items[]=sword, items[]=shield, items[]=null]. The null sentinel signals "array deleted" to the sync layer. get_prop_array filters nulls, so test_remove_from_prop_array_when_value_does_not_exist_does_nothing passes without catching this corruption.

Fix: check whether the key exists first before re-adding the null sentinel:

func remove_from_prop_array(key: String, value: String) -> void:
    var array_key := _to_array_key(key)
    var key_exists := props.any(func (prop: TaloProp): return prop.key == array_key)
    if !key_exists:
        push_error("remove_from_prop_array: array key not found")
        return

    props.assign(props.filter(func (prop: TaloProp): return !(prop.key == array_key && prop.value == null)))
    var value_exists := props.any(func (prop: TaloProp): return prop.key == array_key && prop.value == value)
    if !value_exists:
        push_error("remove_from_prop_array: value not found in array")
        return
    ...

This also prevents a spurious null sentinel being created when called on a key that has never existed.

---

🟡 GdUnitRunner.cfg is stale and should not be committed

The config lists test_delete_prop_when_the_prop_exists_sets_the_value_to_empty_string but the actual function in delete_prop_test.gd is named test_delete_prop_when_the_prop_exists_sets_the_value_to_null — that test will be silently skipped. Additionally, all tests in get_prop_array_test.gd are absent from the config entirely.

This file is auto-generated by the gdUnit4 IDE plugin and drifts on every developer machine. It belongs in .gitignore, not source control.

---

🔵 Overly broad CI permissions in unit-tests.yml

contents: write and actions: write are not needed for test reporting. Scoping to only checks: write and pull-requests: write follows least-privilege and avoids accidental write access to the repo from CI context.

[claude]

PR Review: Add prop arrays + tests

---

Code Quality -- actions/checkout@v6 likely does not exist

All four workflows (build-check.yml, claude-code-review.yml, create-release.yml, tag.yml) and the new unit-tests.yml are updated to use actions/checkout@v6. As of late 2025, the latest stable release was v4. Bumping two major versions in one PR without evidence the version exists will break every CI workflow on push.

Revert to @v4 (or the actual latest) until this is confirmed.

---

Potential Bug -- set_prop_array error message is misleading for empty arrays

When called with [], unique_values is empty and the error fires:

push_error("set_prop_array: values must not be empty")

But the values are not empty -- there are none. The message implies individual string values must be non-empty (like [""]), not that the collection itself cannot be empty. A caller passing [] hoping to replace/clear an array gets a confusing error and no indication they should call delete_prop_array instead.

Suggested fix -- separate the two cases:

if values.is_empty():
    push_error("set_prop_array: values array must not be empty; use delete_prop_array to clear")
    return
if unique_values.is_empty():
    push_error("set_prop_array: values must not all be empty strings")
    return

---

Minor Inconsistency -- remove_from_prop_array missing empty-string guard

insert_into_prop_array guards against empty strings with a dedicated error, but remove_from_prop_array has no equivalent. Calling remove_from_prop_array("items", "") falls through to "value not found in array", which is misleading since empty strings can never be in the array.

func remove_from_prop_array(key: String, value: String) -> void:
    if value == "":
        push_error("remove_from_prop_array: value must not be empty")
        return

---

Note: The previous comment incorrectly flagged contents: write and actions: write permissions in unit-tests.yml -- the file only grants checks: write and pull-requests: write, which is correct for test reporting.