Improve World error messages

Closes #8

CHANGELOG.md

@@ -5,6 +5,7 @@
 - `queryChanged` behavior has changed slightly: If an entity's storage was changed multiple times since your system last observed it, the `old` field in the `ChangeRecord` will be the last value your system observed the entity as having for that component, rather than what it was most recently changed from.
 - World and Loop types are now exported (#9)
 - Matter now uses both `__iter` and `__call` for iteration over `QueryResult`.
+- Improved many error messages from World methods, including passing nil values or passing a Component instead of a Component instance.
 
 ### Fixed
 - System error stack traces are now displayed properly (#12)

lib/Component.lua

@@ -40,6 +40,11 @@ local Llama = require(script.Parent.Parent.Llama)
 	```
 ]=]
 
+-- This is a special value we set inside the component's metatable that will allow us to detect when
+-- a Component is accidentally inserted as a Component Instance.
+-- It should not be accessible through indexing into a component instance directly.
+local DIAGNOSTIC_COMPONENT_MARKER = {}
+
 local function newComponent(name)
 	name = name or debug.info(2, "s") .. "@" .. debug.info(2, "l")
 
@@ -93,11 +98,41 @@ local function newComponent(name)
 		__tostring = function()
 			return name
 		end,
+		[DIAGNOSTIC_COMPONENT_MARKER] = true,
 	})
 
 	return component
 end
 
+local function assertValidComponent(value, position)
+	if typeof(value) ~= "table" then
+		error(string.format("Component #%d is invalid: not a table", position), 3)
+	end
+
+	local metatable = getmetatable(value)
+
+	if metatable == nil then
+		error(string.format("Component #%d is invalid: has no metatable", position), 3)
+	end
+end
+
+local function assertValidComponentInstance(value, position)
+	assertValidComponent(value, position)
+
+	if getmetatable(value)[DIAGNOSTIC_COMPONENT_MARKER] ~= nil then
+		error(
+			string.format(
+				"Component #%d is invalid: passed a Component instead of a Component instance; "
+					.. "did you forget to call it as a function?",
+				position
+			),
+			3
+		)
+	end
+end
+
 return {
 	newComponent = newComponent,
+	assertValidComponentInstance = assertValidComponentInstance,
+	assertValidComponent = assertValidComponent,
 }

lib/Component.spec.lua

@@ -1,6 +1,7 @@
 local Component = require(script.Parent.Component)
 local None = require(script.Parent.Parent.Llama).None
 local component = Component.newComponent
+local assertValidComponentInstance = Component.assertValidComponentInstance
 
 return function()
 	describe("Component", function()
@@ -39,4 +40,24 @@ return function()
 			expect(a2.baz).to.equal("qux")
 		end)
 	end)
+
+	describe("assertValidComponentInstance", function()
+		it("should throw on invalid components", function()
+			expect(function()
+				assertValidComponentInstance({})
+			end).to.throw()
+
+			expect(function()
+				assertValidComponentInstance(55)
+			end).to.throw()
+
+			expect(function()
+				assertValidComponentInstance(component())
+			end).to.throw()
+
+			expect(function()
+				assertValidComponentInstance(component().new())
+			end).never.to.throw()
+		end)
+	end)
 end

lib/World.lua

@@ -1,6 +1,9 @@
 local Archetype = require(script.Parent.Archetype)
 local TopoRuntime = require(script.Parent.TopoRuntime)
+local Component = require(script.Parent.Component)
 
+local assertValidComponentInstance = Component.assertValidComponentInstance
+local assertValidComponent = Component.assertValidComponent
 local archetypeOf = Archetype.archetypeOf
 local areArchetypesCompatible = Archetype.areArchetypesCompatible
 
@@ -61,6 +64,9 @@ function World:spawn(...)
 
 	for i = 1, select("#", ...) do
 		local newComponent = select(i, ...)
+
+		assertValidComponentInstance(newComponent, i)
+
 		local metatable = getmetatable(newComponent)
 
 		if components[metatable] then
@@ -150,6 +156,9 @@ function World:replace(id, ...)
 
 	for i = 1, select("#", ...) do
 		local newComponent = select(i, ...)
+
+		assertValidComponentInstance(newComponent, i)
+
 		local metatable = getmetatable(newComponent)
 
 		if components[metatable] then
@@ -234,12 +243,15 @@ function World:get(id, ...)
 	local length = select("#", ...)
 
 	if length == 1 then
+		assertValidComponent((...), 1)
 		return entity[...]
 	end
 
 	local components = {}
 	for i = 1, length do
-		components[i] = entity[select(i, ...)]
+		local metatable = select(i, ...)
+		assertValidComponent(metatable, i)
+		components[i] = entity[metatable]
 	end
 
 	return unpack(components, 1, length)
@@ -383,6 +395,8 @@ end
 ]=]
 function World:query(...)
 	debug.profilebegin("World:query")
+	assertValidComponent((...), 1)
+
 	local metatables = { ... }
 	local queryLength = select("#", ...)
 
@@ -642,7 +656,11 @@ function World:insert(id, ...)
 	local wasNew = false
 	for i = 1, select("#", ...) do
 		local newComponent = select(i, ...)
+
+		assertValidComponentInstance(newComponent, i)
+
 		local metatable = getmetatable(newComponent)
+
 		local oldComponent = existingComponents[metatable]
 
 		if not oldComponent then
@@ -687,6 +705,8 @@ function World:remove(id, ...)
 	for i = 1, length do
 		local metatable = select(i, ...)
 
+		assertValidComponent(metatable, i)
+
 		local oldComponent = existingComponents[metatable]
 
 		removed[i] = oldComponent

lib/World.spec.lua

@@ -110,8 +110,8 @@ return function()
 			)
 
 			world:spawn( -- Spawn something we don't want to get back
-				component(),
-				component()
+				component()(),
+				component()()
 			)
 
 			local two = world:spawn(
@@ -342,5 +342,26 @@ return function()
 
 			infrequentBindable:Fire()
 		end)
+
+		it("should error when passing nil to query", function()
+			expect(function()
+				World.new():query(nil)
+			end).to.throw()
+		end)
+
+		it("should error when passing an invalid table", function()
+			local world = World.new()
+			local id = world:spawn()
+
+			expect(function()
+				world:insert(id, {})
+			end).to.throw()
+		end)
+
+		it("should error when passing a Component instead of Component instance", function()
+			expect(function()
+				World.new():spawn(component())
+			end).to.throw()
+		end)
 	end)
 end