Changing childspawner:ReleaseAllChildren() to a new function that only calls the original breaks the webby whistle

[oblivioncth]

Not quite sure what's going on here and was hoping someone else might have some insight, less this be an engine bug.

I was looking into why a slightly older mod caused odd behavior with spiders and discovered something very strange. 

The problem is that with the mod active, when the webby whistle is used all of the released spiders perform their hiss/roar animation and then immediately renter the den instead of wandering around for a bit like they're supposed to.

spider_whistle.lua shows that when its used it simply calls SummonChildren which in turn calls childspawner:ReleaseAllChildren(). Using this knowledge with a bit of trial and error I discovered the problematic section of the mod: Use of AddComponentPostInit to change the behavior of ReleaseAllChildren. Removing that section stops the problem (but obviously breaks the mod).

The thing is though, nothing about the section looked like it would be an issue. It simply stores the original function in a local variable and then sets the components function to a new one that conditionally calls the original function in order to enforce custom behavior; a technique I've seen used, and have personally used successfully in various mods. The only thing I could think of is that maybe the function definition had changes since the mod was last update and so the arguments were being passed in the wrong order, but that isn't the case either.

To make a long story short, I was able to confirm with some experimentation that non of the mods original custom logic is to blame and that simply the act of wrapping the original ReleaseAllChildren function is a custom one breaks spider den behavior specifically with the webby whistle (they seem to come out normally during dusk and when a non-Webber player steps on webbing).

You literally can add a mod to your game with this as the sole contents of modmain.lua and it will produce the behavior I describe above, even though as far as I can tell this should effectively do nothing other than make the call slightly less efficient 

function overrideChildSpawner(self)
	local originalReleaseAllChildren = self.ReleaseAllChildren
	
    local function newReleaseAllChildren(inst, target, prefab)
	  originalReleaseAllChildren(inst, target, prefab)
    end
	
    self.ReleaseAllChildren = newReleaseAllChildren
end
AddComponentPostInit("childspawner", overrideChildSpawner)

I'm not particularly experienced with lua, but like I said before this general technique is employed frequently in mods for this game without issue. The only thing that is slightly different is that all uses I've seen before were to replace the functions of individual "class" instances that were clearly designed with reassignment in mind, like AddPrefabPostInit with "armor_sanity" to change its equippable.dapperfn, where as this is changing the base function of an entire component. I suspect this is related but I have no way of knowing for sure.

Does someone know what the deal is here? Perhaps I'm missing something obvious...

EDIT:

The bees and bee boxes/hives seem to perform correctly with this "mod" enabled, tested since they also use childspawner:ReleaseAllChildren(). This strangely seems isolated to the webby whistle.

Edited August 2, 2021 by oblivioncth
Additional information.

[penguin0616]

The problem with the code is in the title: "Changing childspawner:ReleaseAllChildren() to a new function that only calls the original breaks the webby whistle."

The function ReleaseAllChildren is being replaced incorrectly. In addition to calling the original, it needs to return the results of that call.

function overrideChildSpawner(self)
	local originalReleaseAllChildren = self.ReleaseAllChildren
	
	local function newReleaseAllChildren(inst, target, prefab)
		return originalReleaseAllChildren(inst, target, prefab)
	end
	
	self.ReleaseAllChildren = newReleaseAllChildren
end
AddComponentPostInit("childspawner", overrideChildSpawner)

(Note that while the chosen arguments for the new method are fine at the moment, they ideally should be a tuple or include one instead).

Edited August 2, 2021 by penguin0616
Fixed indentation, added minor note.

[oblivioncth]

11 hours ago, penguin0616 said:

-snip-

Wow durp... nice call, literally the one thing I missed.

I originally looked here for an older version of the script to see if the function definition changed:

https://github.com/taichunmin/dont-starve-together-game-scripts/blob/master/components/childspawner.lua

Of course it didn't, but while I did see the function was expanded upon greatly, I didn't think much of it after skimming through and apparently stopped reading a little too soon. As you're obviously aware, the old version didn't return anything, but way down at the bottom of the new one there is indeed

return children_released

Definitely a case where my inexperience with lua directly screwed me over, as I'm much more used to languages with more concrete syntax (C/C++/C#, Java, etc) that require defining a return type and so when I saw that the function parameters didn't change I stopped focusing as much with the rest of the function.

In one of the above languages I would have known right away if the function went from

void ChildSpawner::ReleaseAllChildren(Target target, Prefab prefab);

to

int ChildSpawner::ReleaseAllChildren(Target target, Prefab prefab);

but here with lua I needed to keep going and specifically look for the presence of a return statement to also see if the "return type" changed.

The actual mod does need to make use of the target argument, so with taking your suggestion into account and simplifying a bit this seems to do the trick (unless you meant something else):

AddComponentPostInit("childspawner", function(self)
	local originalReleaseAllChildren = self.ReleaseAllChildren
	
	self.ReleaseAllChildren = function(inst, target, ...)
		-- Actual new handling here
		return originalReleaseAllChildren(inst, target, ...)
	end
end)

Thanks a bunch for catching my mistake.

Edited August 2, 2021 by oblivioncth
Truncation of unnecessary reply text