Merge pull request #7383 from karl-zylinski/handle-map-fixes

Handle Map: Nicer organization + fix broken test + add test

core/container/handle_map/doc.odin

@@ -48,11 +48,11 @@ Example:
 		hm.remove(&entities, h1)
 
 		h3 := hm.add(&entities, Entity{pos = {6, 7}})
-		assert(hm.is_valid(entities, h3))
+		assert(hm.is_valid(&entities, h3))
 
 		it := hm.iterator_make(&entities)
 		for e, h in hm.iterate(&it) {
-			assert(hm.is_valid(entities, h))
+			assert(hm.is_valid(&entities, h))
 			e.pos += {1, 2}
 		}
 	}

core/container/handle_map/general.odin

@@ -0,0 +1,66 @@
+// This file contains pre-defined Handle types. It also contains procedure groups that work with
+// both static and dynamic handle maps.
+package container_handle_map
+
+// Default 16-bit Handle type which can be used for handle maps which only need a maximum of 254 (1<<8 - 2) items
+Handle16 :: struct {
+	idx: u8,
+	gen: u8,
+}
+
+// Default 32-bit Handle type which can be used for handle maps which only need a maximum of 65534 (1<<16 - 2) items
+Handle32 :: struct {
+	idx: u16,
+	gen: u16,
+}
+
+// Default 64-bit Handle type which can be used for handle maps which only need a maximum of 4294967294 (1<<32 - 2) items
+Handle64 :: struct {
+	idx: u32,
+	gen: u32,
+}
+
+add :: proc{
+	static_add,
+	dynamic_add,
+}
+
+get :: proc{
+	static_get,
+	dynamic_get,
+}
+
+remove :: proc{
+	static_remove,
+	dynamic_remove,
+}
+
+is_valid :: proc{
+	static_is_valid,
+	dynamic_is_valid,
+}
+
+len :: proc{
+	static_len,
+	dynamic_len,
+}
+
+cap :: proc{
+	static_cap,
+	dynamic_cap,
+}
+
+clear :: proc{
+	static_clear,
+	dynamic_clear,
+}
+
+iterator_make :: proc{
+	static_iterator_make,
+	dynamic_iterator_make,
+}
+
+iterate :: proc{
+	static_iterate,
+	dynamic_iterate,
+}

core/container/handle_map/static_handle_map.odin

@@ -3,24 +3,6 @@ package container_handle_map
 import "base:builtin"
 import "base:intrinsics"
 
-// Default 16-bit Handle type which can be used for handle maps which only need a maximum of 254 (1<<8 - 2) items
-Handle16 :: struct {
-	idx: u8,
-	gen: u8,
-}
-
-// Default 32-bit Handle type which can be used for handle maps which only need a maximum of 65534 (1<<16 - 2) items
-Handle32 :: struct {
-	idx: u16,
-	gen: u16,
-}
-
-// Default 64-bit Handle type which can be used for handle maps which only need a maximum of 4294967294 (1<<32 - 2) items
-Handle64 :: struct {
-	idx: u32,
-	gen: u32,
-}
-
 Static_Handle_Map :: struct($N: uint, $T: typeid, $Handle_Type: typeid)
 	where
 		0 < N, N < uint(1<<31 - 1),
@@ -173,50 +155,3 @@ static_iterate :: proc "contextless" (it: ^$HI/Static_Handle_Map_Iterator($H/Sta
 	it.index = 0
 	return
 }
-
-
-
-add :: proc{
-	static_add,
-	dynamic_add,
-}
-
-get :: proc{
-	static_get,
-	dynamic_get,
-}
-
-remove :: proc{
-	static_remove,
-	dynamic_remove,
-}
-
-is_valid :: proc{
-	static_is_valid,
-	dynamic_is_valid,
-}
-
-len :: proc{
-	static_len,
-	dynamic_len,
-}
-
-cap :: proc{
-	static_cap,
-	dynamic_cap,
-}
-
-clear :: proc{
-	static_clear,
-	dynamic_clear,
-}
-
-iterator_make :: proc{
-	static_iterator_make,
-	dynamic_iterator_make,
-}
-
-iterate :: proc{
-	static_iterate,
-	dynamic_iterate,
-}

tests/core/container/test_core_handle_map.odin

@@ -58,7 +58,7 @@ test_dynamic_handle_map :: proc(t: ^testing.T) {
 	testing.expect(t, xar.len(dhm.unused_items) == N / 2)
 
 	it := hm.dynamic_iterator_make(&dhm)
-	for v, handle in hm.iterate(&it) {
+	for v, handle in hm.dynamic_iterate(&it) {
 		assert(v.handle.idx & 1 == 1)
 		assert(hm.dynamic_is_valid(&dhm, handle))
 
@@ -77,10 +77,14 @@ test_dynamic_handle_map :: proc(t: ^testing.T) {
 	testing.expect(t, hm.dynamic_cap(dhm) >= N)
 }
 
+@test
 test_static_handle_map :: proc(t: ^testing.T) {
 	N :: 512
 
-	shm: hm.Static_Handle_Map(N, Item, hm.Handle32)
+	// We add `1` because the item at idx == 0 eats one slot. The testing logic below assumes we are
+	// able to add exactly 512 items. But because of the dummy item at the first slot, we run out
+	// of space after 511 items.
+	shm: hm.Static_Handle_Map(N + 1, Item, hm.Handle32)
 
 	items: [dynamic]Item
 	defer delete(items)
@@ -121,7 +125,7 @@ test_static_handle_map :: proc(t: ^testing.T) {
 	testing.expect(t, hm.static_cap(shm) >= N / 2)
 
 	it := hm.static_iterator_make(&shm)
-	for v, handle in hm.iterate(&it) {
+	for v, handle in hm.static_iterate(&it) {
 		assert(v.handle.idx & 1 == 1)
 		assert(hm.static_is_valid(shm, handle))
 
@@ -138,4 +142,71 @@ test_static_handle_map :: proc(t: ^testing.T) {
 	hm.static_clear(&shm)
 	testing.expect(t, hm.static_len(shm) == 0)
 	testing.expect(t, hm.static_cap(shm) == N)
+}
+
+// Same as `test_dynamic_handle_map` but uses the non-specific procedure groups
+@test
+test_dynamic_handle_map_procedure_groups :: proc(t: ^testing.T) {
+	dhm: hm.Dynamic_Handle_Map(Item, hm.Handle32)
+	hm.dynamic_init(&dhm, context.allocator)
+	defer hm.dynamic_destroy(&dhm)
+
+	items: [dynamic]Item
+	defer delete(items)
+
+	N :: 512
+	for i in 1..=N {
+		h, add_err := hm.add(&dhm, Item{v = i * 10 + 1})
+		assert(add_err == nil)
+
+		item := hm.get(&dhm, h)
+		item.handle = h
+		item.p      = item
+		item.my_idx = h.idx
+
+		append(&items, item^)
+	}
+
+	testing.expect(t, hm.len(dhm) == N)
+	testing.expect(t, hm.cap(dhm) >= N)
+
+	for v in items {
+		item := hm.get(&dhm, v.handle)
+		assert(item^ == v)
+
+		// Remove half of the items
+		if item.handle.idx & 1 == 0 {
+			found, found_err := hm.remove(&dhm, v.handle)
+			assert(found && found_err == nil)
+
+			// These removed handles should no longer be valid
+			assert(!hm.is_valid(&dhm, v.handle))
+		} else {
+			// Non-removed handles should still be valid
+			assert(hm.is_valid(&dhm, v.handle))
+		}
+	}
+
+	testing.expect(t, hm.len(dhm) == N / 2)
+	testing.expect(t, hm.cap(dhm) >= N / 2)
+	testing.expect(t, xar.len(dhm.unused_items) == N / 2)
+
+	it := hm.iterator_make(&dhm)
+	for v, handle in hm.iterate(&it) {
+		assert(v.handle.idx & 1 == 1)
+		assert(hm.is_valid(&dhm, handle))
+
+		item := hm.get(&dhm, handle)
+		assert(item.my_idx == v.handle.idx)
+	}
+
+	for i in 1..=N / 2 {
+		h, add_err := hm.add(&dhm, Item{v = i * 10 + 1})
+		assert(add_err == nil)
+		assert(h.gen == 2)
+	}
+
+	hm.clear(&dhm)
+	testing.expect(t, hm.len(dhm) == 0)
+	testing.expect(t, hm.cap(dhm) >= N)
 }