rewrite(phase4): revert changes due to collision and updated tests unit for identifier

src/Util/Identifier.luau

@@ -5,33 +5,38 @@ if not shared.__identifier_cache then
 	shared.__identifier_cache = {
 		cache = {} :: { [string]: number },
 		used = {} :: { [number]: string },
+		count = 0 :: number,
 	}
 end
 
 local registry = shared.__identifier_cache
 local cache = registry.cache
 local used = registry.used
+local count = registry.count
+
+local BITS: number = 16
+local MAX_VALUE: number = bit32.lshift(1, BITS) - 1
+local hash: number = game.PlaceVersion + 5381
 
 return function(name: string): number
 	local cached = cache[name]
 	if cached then
 		return cached
 	end
-
-	local h = 2166136261 + game.PlaceVersion
+	local b: number = hash
 	for i = 1, #name do
-		h = bit32.bxor(h, string.byte(name, i))
-		h = bit32.band(h * 16777619, 0xFFFFFFFF)
+		b = bit32.bxor(bit32.lshift(b, 5) + b, string.byte(name, i))
 	end
-
-	local id = h % 65534 + 2
-
-	local existing = used[id]
+	local reduced = bit32.band(b, MAX_VALUE)
+	if reduced < 2 then
+		reduced += 2
+	end
+	local existing = used[reduced]
 	if existing and existing ~= name then
-		error(`[Warp] Hash collision: "{name}" & "{existing}" both map to ID {id}. Rename one.`)
+		error(`[Warp] Hash collision: "{name}" & "{existing}" both map to ID {reduced}. Rename one.`)
 	end
-
-	cache[name] = id
-	used[id] = name
-	return id
-end
+	used[name] = reduced
+	cache[name] = reduced
+	count += 1
+	return reduced
+end

test/identifier.spec.luau

@@ -4,6 +4,8 @@ return function()
 	local ReplicatedStorage = game:GetService("ReplicatedStorage")
 	local WarpModule = ReplicatedStorage:WaitForChild("Warp")
 	local Identifier = require(WarpModule:WaitForChild("Util"):WaitForChild("Identifier"))
+	
+	local U16_MAX, U16_MIN = 65535, 2
 
 	describe("Warp.Identifier", function()
 		it("returns a number", function()
@@ -16,10 +18,10 @@ return function()
 			expect(result == math.floor(result)).to.equal(true)
 		end)
 
-		it("returns a value within 0-255 range (8-bit)", function()
+		it(`returns a value within {U16_MIN}-{U16_MAX} range (8-bit)`, function()
 			local result = Identifier("TestEvent")
-			expect(result >= 0).to.equal(true)
-			expect(result <= 255).to.equal(true)
+			expect(result >= U16_MIN).to.equal(true)
+			expect(result <= U16_MAX).to.equal(true)
 		end)
 
 		it("is deterministic (same name returns same value)", function()
@@ -39,23 +41,23 @@ return function()
 		it("handles empty string", function()
 			local result = Identifier("")
 			expect(type(result)).to.equal("number")
-			expect(result >= 0).to.equal(true)
-			expect(result <= 255).to.equal(true)
+			expect(result >= U16_MIN).to.equal(true)
+			expect(result <= U16_MAX).to.equal(true)
 		end)
 
 		it("handles single character strings", function()
 			local result = Identifier("A")
 			expect(type(result)).to.equal("number")
-			expect(result >= 0).to.equal(true)
-			expect(result <= 255).to.equal(true)
+			expect(result >= U16_MIN).to.equal(true)
+			expect(result <= U16_MAX).to.equal(true)
 		end)
 
 		it("handles long strings", function()
 			local longName = string.rep("abcdefghij", 100)
 			local result = Identifier(longName)
 			expect(type(result)).to.equal("number")
-			expect(result >= 0).to.equal(true)
-			expect(result <= 255).to.equal(true)
+			expect(result >= U16_MIN).to.equal(true)
+			expect(result <= U16_MAX).to.equal(true)
 		end)
 
 		it("produces at least some distinct values for different names", function()
@@ -64,7 +66,7 @@ return function()
 			for _, name in names do
 				unique[Identifier(name)] = true
 			end
-			local count = 0
+			local count = U16_MIN
 			for _ in unique do
 				count += 1
 			end
@@ -75,8 +77,8 @@ return function()
 		it("stays within 8-bit range across many inputs", function()
 			for i = 1, 500 do
 				local result = Identifier(`Event{i}`)
-				expect(result >= 0).to.equal(true)
-				expect(result <= 255).to.equal(true)
+				expect(result >= U16_MIN).to.equal(true)
+				expect(result <= U16_MAX).to.equal(true)
 				expect(result == math.floor(result)).to.equal(true)
 			end
 		end)
@@ -104,8 +106,8 @@ return function()
 			for _, name in specialNames do
 				local result = Identifier(name)
 				expect(type(result)).to.equal("number")
-				expect(result >= 0).to.equal(true)
-				expect(result <= 255).to.equal(true)
+				expect(result >= U16_MIN).to.equal(true)
+				expect(result <= U16_MAX).to.equal(true)
 			end
 		end)
 
@@ -114,10 +116,10 @@ return function()
 			local b = Identifier("456")
 			expect(type(a)).to.equal("number")
 			expect(type(b)).to.equal("number")
-			expect(a >= 0).to.equal(true)
-			expect(a <= 255).to.equal(true)
-			expect(b >= 0).to.equal(true)
-			expect(b <= 255).to.equal(true)
+			expect(a >= U16_MIN).to.equal(true)
+			expect(a <= U16_MAX).to.equal(true)
+			expect(b >= U16_MIN).to.equal(true)
+			expect(b <= U16_MAX).to.equal(true)
 		end)
 
 		it("different names that are similar still hash correctly", function()
@@ -128,9 +130,9 @@ return function()
 			expect(type(b)).to.equal("number")
 			expect(type(c)).to.equal("number")
 			-- all in range
-			expect(a >= 0 and a <= 255).to.equal(true)
-			expect(b >= 0 and b <= 255).to.equal(true)
-			expect(c >= 0 and c <= 255).to.equal(true)
+			expect(a >= U16_MIN and a <= U16_MAX).to.equal(true)
+			expect(b >= U16_MIN and b <= U16_MAX).to.equal(true)
+			expect(c >= U16_MIN and c <= U16_MAX).to.equal(true)
 		end)
 	end)
 end