From d8ee72569e50727eec509c93e8fd0f29bf2dbefc Mon Sep 17 00:00:00 2001 From: Christian Treffs Date: Thu, 10 May 2018 15:41:33 +0200 Subject: [PATCH] Fix major flaws in SparseSet --- Sources/FirebladeECS/Nexus+Component.swift | 2 +- Sources/FirebladeECS/Nexus+Entity.swift | 6 +- Sources/FirebladeECS/Nexus+Family.swift | 4 +- Sources/FirebladeECS/Nexus.swift | 37 +- Sources/FirebladeECS/SparseSet.swift | 214 +++++++---- Tests/FirebladeECSTests/NexusTests.swift | 59 ++- Tests/FirebladeECSTests/SparseSetTests.swift | 370 +++++++++++++++++-- 7 files changed, 533 insertions(+), 159 deletions(-) diff --git a/Sources/FirebladeECS/Nexus+Component.swift b/Sources/FirebladeECS/Nexus+Component.swift index 6db95ee..50eefb9 100644 --- a/Sources/FirebladeECS/Nexus+Component.swift +++ b/Sources/FirebladeECS/Nexus+Component.swift @@ -47,7 +47,7 @@ extension Nexus { if componentIdsByEntity[entityIdx] == nil { componentIdsByEntity[entityIdx] = SparseComponentIdentifierSet() } - componentIdsByEntity[entityIdx]?.add(componentId, at: componentId.hashValue) + componentIdsByEntity[entityIdx]?.insert(componentId, at: componentId.hashValue) update(familyMembership: entityId) diff --git a/Sources/FirebladeECS/Nexus+Entity.swift b/Sources/FirebladeECS/Nexus+Entity.swift index ebdefa4..698ac04 100644 --- a/Sources/FirebladeECS/Nexus+Entity.swift +++ b/Sources/FirebladeECS/Nexus+Entity.swift @@ -19,7 +19,7 @@ extension Nexus { let newEntityIdentifier: EntityIdentifier = newEntityIndex.identifier let newEntity: Entity = Entity(nexus: self, id: newEntityIdentifier, name: name) - entityStorage.add(newEntity, at: newEntityIndex) + entityStorage.insert(newEntity, at: newEntityIndex) notify(EntityCreated(entityId: newEntityIdentifier)) return newEntity } @@ -30,7 +30,7 @@ extension Nexus { } public func has(entity entityId: EntityIdentifier) -> Bool { - return entityStorage.has(entityId.index) + return entityStorage.contains(entityId.index) } public func get(entity entityId: EntityIdentifier) -> Entity? { @@ -42,7 +42,7 @@ extension Nexus { let entityId: EntityIdentifier = entity.identifier let entityIdx: EntityIndex = entityId.index - guard entityStorage.remove(at: entityIdx) else { + guard entityStorage.remove(at: entityIdx) != nil else { report("EntityRemove failure: no entity \(entityId) to remove") return false } diff --git a/Sources/FirebladeECS/Nexus+Family.swift b/Sources/FirebladeECS/Nexus+Family.swift index af6a55d..6f3d1f3 100644 --- a/Sources/FirebladeECS/Nexus+Family.swift +++ b/Sources/FirebladeECS/Nexus+Family.swift @@ -49,7 +49,7 @@ public extension Nexus { guard let members: UniformEntityIdentifiers = members(of: traits) else { return false } - return members.has(entityId.index) + return members.contains(entityId.index) } } @@ -137,7 +137,7 @@ private extension Nexus { func add(to traits: FamilyTraitSet, entityId: EntityIdentifier, entityIdx: EntityIndex) { createTraitsIfNeccessary(traits: traits) - familyMembersByTraits[traits]?.add(entityId, at: entityIdx) + familyMembersByTraits[traits]?.insert(entityId, at: entityIdx) } func remove(from traits: FamilyTraitSet, entityId: EntityIdentifier, entityIdx: EntityIndex) { diff --git a/Sources/FirebladeECS/Nexus.swift b/Sources/FirebladeECS/Nexus.swift index c47e112..ac6f407 100644 --- a/Sources/FirebladeECS/Nexus.swift +++ b/Sources/FirebladeECS/Nexus.swift @@ -55,24 +55,27 @@ public class Nexus: Equatable { familyMembersByTraits = [:] } + public final func clear() { + for entity: Entity in entityStorage { + destroy(entity: entity) + } + + entityStorage.clear() + freeEntities.removeAll() + + assert(entityStorage.isEmpty) + assert(componentsByType.values.reduce(0) { $0 + $1.count } == 0) + assert(componentIdsByEntity.values.reduce(0) { $0 + $1.count } == 0) + assert(freeEntities.isEmpty) + assert(familyMembersByTraits.values.reduce(0) { $0 + $1.count } == 0) + + componentsByType.removeAll() + componentIdsByEntity.removeAll() + familyMembersByTraits.removeAll() + } + deinit { - - for entity: Entity in entityStorage { - destroy(entity: entity) - } - - entityStorage.clear() - freeEntities.removeAll() - - assert(entityStorage.isEmpty) - assert(componentsByType.values.reduce(0) { $0 + $1.count } == 0) - assert(componentIdsByEntity.values.reduce(0) { $0 + $1.count } == 0) - assert(freeEntities.isEmpty) - assert(familyMembersByTraits.values.reduce(0) { $0 + $1.count } == 0) - - componentsByType.removeAll() - componentIdsByEntity.removeAll() - familyMembersByTraits.removeAll() + clear() } // MARK: Equatable diff --git a/Sources/FirebladeECS/SparseSet.swift b/Sources/FirebladeECS/SparseSet.swift index 3e937dd..e54391d 100644 --- a/Sources/FirebladeECS/SparseSet.swift +++ b/Sources/FirebladeECS/SparseSet.swift @@ -5,119 +5,171 @@ // Created by Christian Treffs on 30.10.17. // -public class SparseSet: UniformStorage, Sequence { - public typealias Index = Int - private typealias DenseIndex = Int - private var size: Int = 0 - private var denseIndices: ContiguousArray - private var denseData: ContiguousArray - private var sparse: [Index: DenseIndex] +public class SparseSet: Sequence { + public typealias Index = Int + public typealias Key = Int + + public struct Entry { + let key: Key + let element: Element + } + + private(set) var dense: ContiguousArray + private(set) var sparse: [Index: Key] // TODO: implement // a) RandomAccessCollection conformance // b) subscript - public init() { - denseIndices = ContiguousArray() - denseData = ContiguousArray() - sparse = [Index: DenseIndex]() - } + public init() { + sparse = [Index: Key]() + dense = ContiguousArray() + } - deinit { - clear() - } + deinit { + clear() + } - public var count: Int { return size } - public var isEmpty: Bool { return size == 0 } + public var count: Int { return dense.count } + public var isEmpty: Bool { return dense.isEmpty } + public var capacity: Int { return sparse.count } - public func has(_ index: Index) -> Bool { - return sparse[index] ?? Int.max < count /*&& - denseIndices[safe: sparse[index]!] != nil*/ - } + public func contains(_ key: Key) -> Bool { + return find(at: key) != nil + } - public func add(_ element: Element, at index: Index) { - if has(index) { - return - } - sparse[index] = count - denseIndices.append(index) - denseData.append(element) - size += 1 - } + /// Inset an element for a given key into the set in O(1). + /// Elements at previously set keys will be replaced. + /// + /// - Parameters: + /// - element: the element + /// - key: the key + /// - Returns: true if new, false if replaced. + @discardableResult + public func insert(_ element: Element, at key: Key) -> Bool { + if let (denseIndex, _) = find(at: key) { + dense[denseIndex] = Entry(key: key, element: element) + return false + } - public func get(at index: Index) -> Element? { - guard let sIdx: Index = sparse[index] else { - return nil - } - return denseData[sIdx] - } + let nIndex = dense.count + dense.append(Entry(key: key, element: element)) + sparse[key] = nIndex + return true + } - @discardableResult - public func remove(at index: Index) -> Bool { - guard has(index), let removeIdx: DenseIndex = sparse[index] else { - return false - } + /// Get the element for the given key in O(1). + /// + /// - Parameter key: the key + /// - Returns: the element or nil of key not found. + public func get(at key: Key) -> Element? { + guard let (_, element) = find(at: key) else { + return nil + } - let lastIdx: DenseIndex = count - 1 - denseIndices.swapAt(removeIdx, lastIdx) - denseData.swapAt(removeIdx, lastIdx) - sparse[index] = nil - let swappedIndex: Index = denseIndices[removeIdx] - sparse[swappedIndex] = removeIdx - denseIndices.removeLast() - denseData.removeLast() - size -= 1 - if size == 0 { - clear(keepingCapacity: false) - } - return true - } + return element + } - public func clear(keepingCapacity: Bool = false) { - size = 0 - denseIndices.removeAll(keepingCapacity: keepingCapacity) - denseData.removeAll(keepingCapacity: keepingCapacity) - sparse.removeAll(keepingCapacity: keepingCapacity) - } + /// Removes the element entry for given key in O(1). + /// + /// - Parameter key: the key + /// - Returns: removed value or nil if key not found. + @discardableResult + public func remove(at key: Key) -> Entry? { + guard let (denseIndex, _) = find(at: key) else { - public func makeIterator() -> SparseSetIterator { - return SparseSetIterator(self) - } + return nil + } - // MARK: - SparseIterator - public struct SparseSetIterator: IteratorProtocol { - private let sparseSet: SparseSet - private var iterator: IndexingIterator> + let removed = swapRemove(at: denseIndex) + if !dense.isEmpty && denseIndex < dense.count { + let swappedElement = dense[denseIndex] + sparse[swappedElement.key] = denseIndex + } + sparse[key] = nil + return removed + } - init(_ sparseSet: SparseSet) { - self.sparseSet = sparseSet - self.iterator = sparseSet.denseData.makeIterator() - } + public func clear(keepingCapacity: Bool = false) { + sparse.removeAll(keepingCapacity: keepingCapacity) + dense.removeAll(keepingCapacity: keepingCapacity) + } - mutating public func next() -> Element? { - return iterator.next() - } + public func makeIterator() -> SparseSetIterator { + return SparseSetIterator(self) + } - } + /// Removes an element from the set and retuns it in O(1). + /// The removed element is replaced with the last element of the set. + /// + /// - Parameter denseIndex: the dense index + /// - Returns: the element entry + private func swapRemove(at denseIndex: Int) -> Entry { + dense.swapAt(denseIndex, dense.count - 1) + return dense.removeLast() + } + + private func find(at key: Key) -> (Int, Element)? { + guard let denseIndex = sparse[key], denseIndex < count else { + return nil + } + let entry = self.dense[denseIndex] + guard entry.key == key else { + return nil + } + + return (denseIndex, entry.element) + } + + // MARK: - SparseIterator + public struct SparseSetIterator: IteratorProtocol { + private let sparseSet: SparseSet + private var sortedSparseIterator: IndexingIterator<[(key: SparseSet.Index, value: SparseSet.Key)]> + + init(_ sparseSet: SparseSet) { + self.sparseSet = sparseSet + + let sortedSparse = sparseSet.sparse.sorted { first, next -> Bool in + first.key < next.key + } + + sortedSparseIterator = sortedSparse.makeIterator() + } + + mutating public func next() -> Element? { + guard let (key, _) = sortedSparseIterator.next() else { + return nil + } + + return sparseSet.get(at: key) + + } + + } +} + +extension SparseSet.Entry: Equatable where SparseSet.Element: Equatable { + public static func == (lhs: SparseSet.Entry, rhs: SparseSet.Entry) -> Bool { + return lhs.element == rhs.element && lhs.key == rhs.key + } } // MARK: - Equatable extension SparseSet: Equatable where SparseSet.Element: Equatable { public static func == (lhs: SparseSet, rhs: SparseSet) -> Bool { - return lhs.denseIndices == rhs.denseIndices && - lhs.denseData == rhs.denseData && - lhs.sparse == rhs.sparse + return lhs.dense == rhs.dense && + lhs.sparse == rhs.sparse } } // MARK: - specialized sparse sets public class SparseEntitySet: SparseSet { - public typealias Index = EntityIndex + public typealias Index = EntityIndex } public class SparseEntityIdentifierSet: SparseSet { - public typealias Index = EntityIndex + public typealias Index = EntityIndex } diff --git a/Tests/FirebladeECSTests/NexusTests.swift b/Tests/FirebladeECSTests/NexusTests.swift index cb0d498..b7bb5ae 100644 --- a/Tests/FirebladeECSTests/NexusTests.swift +++ b/Tests/FirebladeECSTests/NexusTests.swift @@ -10,36 +10,60 @@ import XCTest class NexusTests: XCTestCase { + var nexus: Nexus! + override func setUp() { super.setUp() + nexus = Nexus() } override func tearDown() { + nexus = nil super.tearDown() } - func testCreateEntity() { - let nexus: Nexus = Nexus() - XCTAssert(nexus.numEntities == 0) + func testEntityCreate() { + XCTAssertEqual(nexus.numEntities, 0) let e0 = nexus.create() - XCTAssert(e0.identifier.index == 0) - XCTAssert(nexus.numEntities == 1) + + XCTAssertEqual(e0.identifier.index, 0) + XCTAssertEqual(nexus.numEntities, 1) + + let e1 = nexus.create(entity: "Entity 1") - let e1 = nexus.create(entity: "Named e1") - XCTAssert(e1.identifier.index == 1) - XCTAssert(nexus.numEntities == 2) - - XCTAssert(e0.name == nil) - XCTAssert(e1.name == "Named e1") - - let rE0 = nexus.get(entity: e0.identifier)! - XCTAssert(rE0.name == e0.name) - XCTAssert(rE0.identifier == e0.identifier) + XCTAssert(e1.identifier.index == 1) + XCTAssert(nexus.numEntities == 2) + + XCTAssertNil(e0.name) + XCTAssertEqual(e1.name, "Entity 1") } + + func testEntityDestroy() { + testEntityCreate() + XCTAssertEqual(nexus.numEntities, 2) + + let e1: Entity = nexus.get(entity: 1)! + XCTAssertEqual(e1.identifier.index, 1) + + XCTAssertTrue(nexus.destroy(entity: e1)) + XCTAssertFalse(nexus.destroy(entity: e1)) + + XCTAssertEqual(nexus.numEntities, 1) + + let e1Again: Entity? = nexus.get(entity: 1) + XCTAssertNil(e1Again) + + XCTAssertEqual(nexus.numEntities, 1) + + nexus.clear() + + XCTAssertEqual(nexus.numEntities, 0) + } + func testComponentCreation() { - let nexus: Nexus = Nexus() + XCTAssert(nexus.numEntities == 0) let e0: Entity = nexus.create(entity: "e0") @@ -58,7 +82,7 @@ class NexusTests: XCTestCase { } func testComponentDeletion() { - let nexus = Nexus() + let identifier: EntityIdentifier = nexus.create(entity: "e0").identifier let e0 = nexus.get(entity: identifier)! @@ -106,7 +130,6 @@ class NexusTests: XCTestCase { } func testComponentUniqueness() { - let nexus = Nexus() let a = nexus.create() let b = nexus.create() let c = nexus.create() diff --git a/Tests/FirebladeECSTests/SparseSetTests.swift b/Tests/FirebladeECSTests/SparseSetTests.swift index 3f79a71..eaa7777 100644 --- a/Tests/FirebladeECSTests/SparseSetTests.swift +++ b/Tests/FirebladeECSTests/SparseSetTests.swift @@ -10,13 +10,25 @@ import XCTest class SparseSetTests: XCTestCase { + var set: SparseSet! + + override func setUp() { + super.setUp() + set = SparseSet() + } + + override func tearDown() { + set = nil + super.tearDown() + } + func testSparseSetAdd() { - let set = SparseSet() + let num: Int = 100 for idx in 0..() + func testSparseSetAddAndReplace() { + let p1 = Position(x: 1, y: 1) let p2 = Position(x: 2, y: 2) - set.add(p1, at: 10) + XCTAssertTrue(set.insert(p1, at: 10)) XCTAssertEqual(set.get(at: 10)?.x, p1.x) XCTAssertEqual(set.count, 1) - set.add(p2, at: 10) + XCTAssertFalse(set.insert(p2, at: 10)) - XCTAssertEqual(set.get(at: 10)?.x, p1.x) + XCTAssertEqual(set.get(at: 10)?.x, p2.x) XCTAssertEqual(set.count, 1) } + func testSparseSetGet() { + + let p1 = Position(x: 1, y: 1) + + set.insert(p1, at: 10) + + XCTAssertEqual(set.get(at: 10)?.x, p1.x) + + XCTAssertNil(set.get(at: 33)) + + XCTAssertNotNil(set.remove(at: 10)) + + XCTAssertNil(set.get(at: 10)) + } + func testSparseSetRemove() { - let set = SparseSet() - let num: Int = 100 + + let num: Int = 7 for idx in 0.. = [0, 30, 1, 21, 78, 56, 99, 3] + + for idx in indices { + let pos = Position(x: idx, y: idx) + set.insert(pos, at: idx) + } + + XCTAssertEqual(set.count, indices.count) + XCTAssertEqual(set.sparse.count, indices.count) + XCTAssertEqual(set.dense.count, indices.count) + XCTAssertFalse(set.isEmpty) + + XCTAssertEqual(set.get(at: 0)?.x, 0) + XCTAssertEqual(set.get(at: 30)?.x, 30) + XCTAssertEqual(set.get(at: 1)?.x, 1) + XCTAssertEqual(set.get(at: 21)?.x, 21) + XCTAssertEqual(set.get(at: 78)?.x, 78) + XCTAssertEqual(set.get(at: 56)?.x, 56) + XCTAssertEqual(set.get(at: 99)?.x, 99) + XCTAssertEqual(set.get(at: 3)?.x, 3) + + } func testSparseSetRemoveNonPresent() { - let set = SparseSet() + XCTAssertTrue(set.isEmpty) - XCTAssertFalse(set.remove(at: 100)) + XCTAssertNil(set.remove(at: 100)) XCTAssertTrue(set.isEmpty) } + func testSparseSetDoubleRemove() { + class AClass { } + let set = SparseSet() + let a = AClass() + let b = AClass() + set.insert(a, at: 0) + set.insert(b, at: 1) + + XCTAssertEqual(set.sparse.count, 2) + XCTAssertEqual(set.dense.count, 2) + + XCTAssertEqual(set.count, 2) + + XCTAssertTrue(set.get(at: 0) === a) + XCTAssertTrue(set.get(at: 1) === b) + + XCTAssertEqual(set.count, 2) + + XCTAssertNotNil(set.remove(at: 1)) + + XCTAssertEqual(set.count, 1) + XCTAssertEqual(set.sparse.count, 1) + XCTAssertEqual(set.dense.count, 1) + + + XCTAssertNil(set.remove(at: 1)) + + XCTAssertEqual(set.count, 1) + XCTAssertEqual(set.sparse.count, 1) + XCTAssertEqual(set.dense.count, 1) + + + XCTAssertTrue(set.get(at: 0) === a) + + XCTAssertEqual(set.count, 1) + + } + func testSparseSetNonCongiuousData() { - let set = SparseSet() + var indices: Set = [0, 30, 1, 21, 78, 56, 99, 3] for idx in indices { let pos = Position(x: idx, y: idx) - set.add(pos, at: idx) + set.insert(pos, at: idx) } XCTAssertEqual(set.count, indices.count) - + func recurseValueTest() { for idx in indices { XCTAssertEqual(set.get(at: idx)?.x, idx) XCTAssertEqual(set.get(at: idx)?.y, idx) } } - + recurseValueTest() while let idx = indices.popFirst() { - set.remove(at: idx) + let entry = set.remove(at: idx)! + XCTAssertEqual(entry.key, idx) recurseValueTest() XCTAssertEqual(set.count, indices.count) } @@ -112,7 +404,7 @@ class SparseSetTests: XCTestCase { } func testSparseSetClear() { - let set = SparseSet() + let num: Int = 100 XCTAssertEqual(set.count, 0) @@ -120,7 +412,7 @@ class SparseSetTests: XCTestCase { for idx in 0..() - characters.add("H", at: 4) - characters.add("e", at: 13) - characters.add("l", at: 34) - characters.add("l", at: 44) - characters.add("o", at: 55) - characters.add(" ", at: 66) - characters.add("W", at: 77) - characters.add("o", at: 89) - characters.add("r", at: 90) - characters.add("l", at: 123) - characters.add("d", at: 140) + characters.insert("H", at: 4) + characters.insert("e", at: 13) + + characters.insert("l", at: 44) + characters.insert("o", at: 89) + + characters.insert(" ", at: 66) + characters.insert("d", at: 140) + characters.insert("W", at: 77) + + characters.insert("r", at: 90) + characters.insert("l", at: 123) + characters.insert("o", at: 55) + characters.insert("l", at: 34) + XCTAssertEqual(characters.count, 11)