Why am I getting "C stack overflow"?

I’m creating my own Vector3 class just for fun, but I am having problems with it.

Here is my code

local Vector3 = {};
local mt = {
	__index = Vector3,
	__metatable = "This metatable is locked",
	__newindex = function(t, k, v)
		rawset(t, k, nil);
		error(k .. " is not a valid member of Vector3", 2);
	end,
	__mul = function(op1, op2)
		if (rawequal(type(op1), "table") and rawequal(type(op2), "table")) then
			return Vector3.new(op1.X*op2.X, op1.Y*op2.Y, op1.Z*op2.Z);
		end
		
		if (rawequal(type(op2), "number")) then
			return Vector3.new(op1.X*op2, op1.Y*op2, op1.Z*op2);
		end
	end,
	__tostring = function(t)
		return t.X .. ", " .. t.Y .. ", " .. t.Z;
	end,
	__unm = function(t)
		return Vector3.new(-t.X, -t.Y, -t.Z);
	end,
	__div = function(op1, op2)
		if (rawequal(type(op1), "table") and rawequal(type(op2), "table")) then
			return Vector3.new(op1.X/op2.X, op1.Y/op2.Y, op1.Z/op2.Z);
		end
		
		if (rawequal(type(op2), "number")) then
			return Vector3.new(op1.X/op2, op1.Y/op2, op1.Z/op2);
		end
	end,
	__add = function(op1, op2)
		return Vector3.new(op1.X + op2.X, op1.Y + op2.Y, op1.Z + op2.Z);
	end,
	__sub = function(op1, op2)
		return Vector3.new(op1.X - op2.X, op1.Y - op2.Y, op1.Z + op2.Z)
	end
};

function Vector3.new(x, y, z)
	local self = {};
	self.X = x or 0;
	self.Y = y or 0;
	self.Z = z or 0;
	self.Magnitude = math.sqrt(x^2 + y^2 + z^2);
	setmetatable(self, mt);
	self.Unit = self/self.Magnitude;
	return self;
end

local myVector3 = Vector3.new(10, 20, 30);

print(
	myVector3,
	-myVector3,
	myVector3*2,
	myVector3/2,
	myVector3 + Vector3.new(1, 1, 1),
	myVector3 - Vector3.new(1, 1, 1),
	myVector3*Vector3.new(2, 2, 2),
	myVector3/Vector3.new(2, 2, 2)
);

(I get this error on line 48, which is in the Vector3.new constructor, which is self.Unit = self/self.Magnitude;

From what I understand, this “C stack overflow” would only occur if you invoked the metamethod within the function. Here is an example of what I mean:

t[k] invokes __index again, thus calling that function again, and again: it is a circle.

I am not dividing my table within the __div function? If I am, can someone show me where? Or can other things cause this “C stack overflow” as well that I am doing?

Everything else seems to work just fine.

2 Likes

C stack overflow in simpler terms is pretty much stating your overflowing the stack with repetition, e.g: a infinite loop.
Your dividing self with magnitude, but it returns with another Vector3.new call which in turns calls itself over and over each time unit is set

                if (rawequal(type(op1), "table") and rawequal(type(op2), "table")) then
			return Vector3.new(op1.X/op2.X, op1.Y/op2.Y, op1.Z/op2.Z);
		end
		
		if (rawequal(type(op2), "number")) then
			return Vector3.new(op1.X/op2, op1.Y/op2, op1.Z/op2);
		end

( You call Vector3.new over and over again in your example, self.Unit = self/self.Magnitude )

7 Likes

Ah I see. So basically I was calling Vector3.new within itself without knowing. This makes sence.

How could I avoid this? I mean I need to be able to divide by other numbers and vector3’s as well.

1 Like

Maybe have .Unit be calculated each time (instead of in the constructor). This would obviously make it more expensive, but it wouldn’t be a huge deal. Another thing you could do is make another constructor just for unit vectors but that seems ugly.

Edit: A third option would be to do this:
self.Unit = self.Magnitude == 1 and self or self/self.Magnitude

1 Like

Do the math manually without calling Vector3.new again, create another function that isn’t affected by the metamethod infinite loop, or add a special argument to your custom Vector3 function that states not to repeat itself. (The fastest would be to do the math manually outside of the function call.)

2 Likes

I like the idea of not calculating .Unit each time a new vector is created because it is a fairly expensive operation and doesn’t need to be performed for every vector. Here is an __index metamethod that would do that:

function mt:__index(key)
    local value
    if key == 'Magnitude' then
        value = math.sqrt(self.X^2 + self.Y^2 + self.Z^2)
    elseif key == 'Unit' then
        value = self / self.Magnitude
    end
    self[key] = value
    return value
end

Just make sure not to set __index in the metatable to Vector3 and don’t set .Unit and .Magnitude in the constructor.

4 Likes

I was able to fix the code:

self.Unit = {
		X = self.X/self.Magnitude,
		Y = self.Y/self.Magnitude,
		Z = self.Z/self.Magnitude
	};
	setmetatable(self, mt);
	setmetatable(self.Unit, mt);

@KiIIa_Queen, @Dollar500, @IdiomicLanguage, thank you for your input

1 Like

That may work in this case but in general creating an object without using the constructor is a very bad practice. The problem is that all the computation done in the constructor isn’t being done, and if you were to add something important to the constructor later then the unit vectors wouldn’t have it either. In this case, unit vectors don’t have a magnitude or unit property, which could be easily fixed but you still have to be super careful when editing the constructor. It increases the number of things you need to remember when maintaining or building upon code and leads to bugs.

The idea of adding a different constructor for unit vectors isn’t too bad, but does create duplicate code. That is another reason why I liked the idea of making the Unit and Magnitude properties computed as needed.

3 Likes

Thank you, but this works for now. If need be, I will make the necessary changes.

Small clarification that infinite loops can not cause stack overflows. What should be referred to instead is infinite or unchecked recursion.

3 Likes

Well, I am really late to this discussion but I didnt reslly understand what you meant by that last part

If you can see this then I’d be glad.

I’d be happy to elaborate:

The metamethod I introduced should replace the current __index in the metatable, which is Vector3. Currently, Vector3 didn’t have any values which should be accessible from every vector instance (.new should not belong to every vector in proper OOP). The .Unit and .Magnitude properties should not be set in the constructor (the .new method) otherwise the index metamethod would not be triggered when accessed. Using the __index metamethod to perform and cache the result will delay the calculation until the key is accessed. Thus, a new vector can be created for the .Unit property preserving immutability, and unit vector creation is delayed to access instead of construction to prevent a recursive stack overflow.

1 Like

Oh, so in this case the __index metamethod inside the Vector3 metatable should be used to make the properties?