Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions wurst/data/KeyedSet.wurst
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package KeyedSet
import KeyedTable

/**
A set with O(1) membership, for replacing a `group` used purely to answer "is this unit in here?".

On Lua the elements are the keys of a native Lua table, so `contains` is one hashed index and Lua
does the hashing. That is why this uses new generics (`T:`): they are erased on Lua, so the
element arrives as itself. The old `<T>` containers - `HashSet`, `HashList`, `HashMap` - erase to
`int` instead, so every element round-trips through `castTo int` and back, and an integer index
cannot be a native Lua key at all. Those types also reach the Jass hashtable natives, which the
Jass-Lua shim emulates, paying for a hashtable on a runtime that already is one.

On Jass it falls back to a `Table` keyed by an integer projection of the element. That is a
hashtable native per operation and much slower than the Lua path, but the semantics are the same,
so a package using this works on both backends. Element types with no stable integer key - real,
boolean, string, code, tuples - are a compile error on Jass rather than a lossy key.

**There is no iteration.** Enumerating a Lua table needs `pairs()`, whose order follows internal
hash layout and so differs between clients, which desyncs a lockstep game. `ipairs()` is safe but
only walks consecutive integer keys from 1, so it sees nothing in a table keyed by elements. If
you need to iterate, keep an `ArrayList` alongside and walk that, or use `SparseSet`, whose dense
half exists for exactly this.

**Null is not a valid element**, and **an element must be removed before it is destroyed** - see
`KeyedTable` for why the Jass fallback cannot detect a handle id which has been reused since. This
is the same discipline a `group` needs.
*/
public class KeyedSet<T:>
/** The keyed table whose keys are the elements. */
private int keyTable = 0
/** Tracked rather than measured - counting a Lua table would mean iterating it. */
private int count = 0

construct()
keyTable = keyedTableCreate()

ondestroy
keyedTableDestroy(keyTable)

/** Adds `value`. Returns whether it was newly inserted. */
function add(T value) returns boolean
if keyedTableContains(keyTable, value)
return false
keyedTableAdd(keyTable, value)
count += 1
return true

/** Whether `value` is present. */
function contains(T value) returns boolean
return keyedTableContains(keyTable, value)

/** Removes `value`. Returns whether it was present. */
function remove(T value) returns boolean
if not keyedTableContains(keyTable, value)
return false
keyedTableRemove(keyTable, value)
count -= 1
return true

function size() returns int
return count

function isEmpty() returns boolean
return count == 0

/**
* Empties the set.
*
* Replaces the table rather than clearing it in place: emptying a Lua table means visiting
* its keys, and that needs `pairs()`. The old table is destroyed, which on Jass returns its
* `Table` to the pool and on Lua leaves it to the collector.
*/
function clear()
keyedTableDestroy(keyTable)
keyTable = keyedTableCreate()
count = 0
73 changes: 73 additions & 0 deletions wurst/data/KeyedSetTests.wurst
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package KeyedSetTests

import KeyedSet

class Marker
int id

construct(int id)
this.id = id

@Test
function testAddAndMembership()
let set = new KeyedSet<int>()
set.contains(4).assertFalse()
set.add(4).assertTrue()
set.add(9).assertTrue()
set.contains(4).assertTrue()
set.contains(9).assertTrue()
set.contains(5).assertFalse()
set.size().assertEquals(2)
destroy set

@Test
function testAddIsIdempotent()
let set = new KeyedSet<int>()
set.add(7).assertTrue()
set.add(7).assertFalse()
set.size().assertEquals(1)
set.contains(7).assertTrue()
destroy set

@Test
function testRemoval()
let set = new KeyedSet<int>()
set.add(1)
set.add(2)
set.remove(1).assertTrue()
set.contains(1).assertFalse()
set.contains(2).assertTrue()
set.size().assertEquals(1)
// Removing something absent is not an error and does not move the count.
set.remove(1).assertFalse()
set.size().assertEquals(1)
destroy set

@Test
function testClear()
let set = new KeyedSet<int>()
set.add(1)
set.add(2)
set.clear()
set.isEmpty().assertTrue()
set.size().assertEquals(0)
set.contains(1).assertFalse()
set.add(1).assertTrue()
set.size().assertEquals(1)
destroy set

/** Reference identity, which is what a group-style membership set wants. */
@Test
function testReferenceIdentity()
let set = new KeyedSet<Marker>()
let a = new Marker(1)
let b = new Marker(1)
set.add(a).assertTrue()
set.contains(a).assertTrue()
// Same contents, different object: a distinct member.
set.contains(b).assertFalse()
set.add(b).assertTrue()
set.size().assertEquals(2)
destroy set
destroy a
destroy b
89 changes: 89 additions & 0 deletions wurst/data/KeyedTable.wurst
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package KeyedTable
import Table

// Keyed membership: a table whose keys are the elements themselves.
//
// These are compiler intrinsics, and each backend gets the representation that suits it.
//
// On **Lua** every operation lowers to a single table operation - `{}`, `t[k] = true`,
// `t[k] ~= nil`, `t[k] = nil` - so membership costs one hashed index and Lua does the hashing.
// The key is a `T:` type parameter, which new generics erase on Lua rather than routing through
// `castTo int` the way the old `<T>` containers do, so the element arrives as itself and becomes
// the table key directly. An integer index would defeat native hashing entirely.
//
// On **Jass** there is no hashing, so the bodies below run instead: the element is projected to an
// integer key by `wurstKeyOf` and stored in a `Table`. That costs a hashtable native per operation,
// which is far slower than the Lua path - but it works, which is what a fallback has to do.
//
// **There is no iteration.** Enumerating a Lua table needs `pairs()`, whose order follows internal
// hash layout and so differs between clients, which desyncs a lockstep game. Anything that must be
// iterated needs a separately maintained insertion-ordered array - see `SparseSet`.
//
// **Null is not a valid key.** On Lua `nil` cannot be a table key at all, so adding one is a
// runtime error there; on Jass it would collide with the key reserved for absence. Neither backend
// is asked to invent a meaning for it, and no check is added on the membership path to look for it.
//
// **Remove a handle before destroying it.** Warcraft reuses handle ids. On Jass a key is that id
// and the table stores only that the id is present, so a new handle which inherits the id of a
// destroyed member is reported as a member. Validating that would mean storing the element next to
// its key, and `Table` offers no way to store an arbitrary `T` - only `saveUnit`, `saveItem` and
// the rest, one per concrete type. That is the reason `SparseSet` asks for a `SparseSetKey` and
// keeps its elements in a dense list: it can compare the stored element and this cannot.
//
// The discipline is the same one a `group` needs, and the same one `SparseSet` documents: take an
// element out before it is destroyed. What is not claimed is that the two backends agree about a
// member which was destroyed and whose id has since been reused.

/** A new, empty keyed table. */
@compilerintrinsic public function keyedTableCreate() returns int
return (new Table()) castTo int

/** Adds `key`. Adding a key that is already present has no effect. */
@compilerintrinsic public function keyedTableAdd<T:>(int keyedTable, T key)
(keyedTable castTo Table).saveBoolean(wurstKeyOf(key), true)

/** Whether `key` is present. */
@compilerintrinsic public function keyedTableContains<T:>(int keyedTable, T key) returns boolean
return (keyedTable castTo Table).loadBoolean(wurstKeyOf(key))

/** Removes `key`. Removing a key that is absent has no effect. */
@compilerintrinsic public function keyedTableRemove<T:>(int keyedTable, T key)
(keyedTable castTo Table).removeBoolean(wurstKeyOf(key))

/**
Frees the keyed table.

On Lua this is a no-op: the table is garbage once the last reference is dropped. On Jass it
releases the `Table` instance, which comes from a finite pool - without this, every discarded
keyed structure would burn one permanently.
*/
@compilerintrinsic public function keyedTableDestroy(int keyedTable)
destroy (keyedTable castTo Table)

// ---
// Key projection. Compiler-owned: these exist so the Jass bodies above have something to call,
// and are not meant to be called directly.
// ---

/**
The integer key of `value` on Jass.

A `T:` type parameter cannot be projected to an integer in Wurst - that is why `SparseSet` has to
ask its caller for a `SparseSetKey`. The compiler fills this in after generic elimination, when
each specialisation's element type is concrete, choosing one of the projections below. Element
types with no stable integer key - real, boolean, string, code, tuples - are rejected there with
a compile error rather than keyed on something lossy.

On Lua this is never reached: the operations above are replaced wholesale and the element is its
own key, so there is no projection to make.
*/
@compilerintrinsic public function wurstKeyOf<T:>(T value) returns int
return 0

/** The projection chosen for ints and for class instances, which are integers by that point. */
@compilerintrinsic public function keyOfInt(int value) returns int
return value

/** The projection chosen for handles, whose id is their key. */
@compilerintrinsic public function keyOfHandle(handle value) returns int
return GetHandleId(value)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve handle identity across recycled IDs

When a unit or destructable is destroyed while still present and Warcraft later reuses its handle ID, the Jass table still contains true under that integer, so contains(newHandle) incorrectly returns true and add(newHandle) returns false; the Lua backend instead keys by the handle itself. The existing SparseSet explicitly compares the stored handle to prevent this inheritance (SparseSet.wurst lines 39-45 and 145-148), so the Jass fallback must retain enough identity to validate a reused ID rather than storing only a boolean.

AGENTS.md reference: AGENTS.md:L12-L12

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Jass half of this is real and I have documented it as a contract rather than claimed to fix it. Flagging that choice rather than resolving quietly, because it is a design decision.

Why not fixed. Validating a reused id means storing the element beside its key so it can be compared. Table has no way to store an arbitrary T — it offers saveUnit, saveItem, saveDestructable, saveWidget, savePlayer and so on, one per concrete type, and nothing generic. That is exactly why SparseSet asks its caller for a SparseSetKey and keeps a dense ArrayList<T>: it can compare the stored element, and a keyed table storing one boolean cannot. Adding that here would rebuild SparseSet's shape and give up the reason this type exists.

What I am not claiming. I have not verified how Warcraft represents a handle in Lua, so I will not assert either that the backends agree here or that they differ. What the docs now say is narrower and checkable: an element must be removed before it is destroyed, and membership of a destroyed element whose id has since been reused is not defined. That is the same discipline a group needs and the same one SparseSet already documents.

Documented on both KeyedTable and KeyedSet in 59d4153, including why the fallback cannot do better and where to go instead when identity across recycling matters.

If the view is that a set replacing a group must survive recycling, that is a different type — SparseSet's shape, with the allocation and indirection that come with it — and worth deciding deliberately rather than folding in here.