Revolutionary-Games / Thrive

The main repository for the development of the evolution game Thrive.
https://revolutionarygamesstudio.com/
Other
2.87k stars 506 forks source link

Digested cells sometimes die when they are digested and drop chunks #5231

Open hhyyrylainen opened 5 months ago

hhyyrylainen commented 5 months ago

(instead of having been fully cleared of resources)

Instead of this buggy behaviour it would be better to make sure engulfed things are always fully eaten. I'm not sure what causes this but it could be that things produce more resources while they are being digested and that isn't taken into account in the total resources to digest meaning that digestion is considered complete even when compounds exist.

hhyyrylainen commented 3 months ago

Comment from a player on Discord, that is likely related to the real root cause at play here:

Cells aren't properly digested currently, no. The game will tick down the digested matter and give you resources as though they're being digested, but the cells will stay there until their individual health actually runs down to zero and then eject their organelles. Right now it's basically a resource duplication glitch.

https://discord.com/channels/228300288023461893/228300288023461893/1255225896617644092

MalyshevArtem commented 3 months ago

Can I try to solve it?

hhyyrylainen commented 3 months ago

Yeah, you can give this a go but I'll warn that this may be really hard to pinpoint the problem and then fix it.

MalyshevArtem commented 3 months ago

@hhyyrylainen I've quickly walked through the code in the EngulfedDigestionSystem to get some understanding of how it works.

While digesting a cell an engulfer is not only eating compounds stored in the engulfed cell but also AdditionalEngulfableCompounds calculated based on organelles the engulfed one has. Each update the amounts of compounds in the compound storage and additional compounds are decreasing. At the end of the digestion they are almost zero.

Then after the EngulfedDigestionSystem the MicrobeDeathSystem is called. In this system both types of compounds were empty. There is a method CalculateBonusDigestibleGlucose which adds some glucose. In my case it became 3.125. Then chunks to spawn are calculated based on HexCount. In my case it's 8 chunks. After that there is a loop which spawns chunks. In this loop a randomized compound amount is calculated for each compound in one chunk. For glucose it can be 1.5 or 2 or 3. As I wrote other compounds were zero. So assuming it's always 1.5 it would be 1.5 times 8 or 12. Assuming it's always 3 it would be 24, the amount of glucose to spawn.

After that I didn't find any other obvious ways to spawn something.

As I can understand the problem is that sometimes after the EngulfedDigestionSystem the compounds for the engulfed cell in the MicrobeDeathSystem are not 0 like in my case described.

hhyyrylainen commented 3 months ago

I didn't fully follow what you are getting at... The MicrobeDeathSystem shouldn't process digested cells as normal deaths. That part of your message already sounds like the bug: the digested cell should be digested "properly" so that it is fully destroyed and the microbe death system wouldn't play the death sound or try to spawn chunks in the first place.

There should already be code in the death system to only partially process digested cells. So to me it kind of sounds like the bug is either in how the digested system sets the digested status, how the death system detects what is digested, or a timing problem between those two actions.

If I remember right the reason why the death system even processes digested things is to calculate things like population penalties from dying.

MalyshevArtem commented 3 months ago

@hhyyrylainen Then I'm gonna look at this more carefully again to see why the MicrobeDeathSystem is executed for the cell which has been digested.

I just engulfed a microbe, put a breakpoint in HandleDigestion, found my cell because it has a player mark, looked at what it had as engulfed objects, saw the microbe I'd got and the id of it. Then I waited until it was almost digested and put a breakpoint again to see the amounts of compounds. They were almost done. Put a breakpoint in the HandleMicrobeDeath of the MicrobeDeathSystem. Waited until the cell was fully digested and then the breakpoint was met. It was the same id of a cell. And while debugging next it was going through the loops to spawn chunks in SpawnCorpseChunks.

hhyyrylainen commented 3 months ago

I have a slight feeling that this bit of code is missing for some reason from the microbe death system (I would be the one to cause this bug when porting everything to ECS architecture):

kuva

MalyshevArtem commented 3 months ago

As I can understand it only solves the bug with producing bonus digestible glucose but not solve this main issue or not? Because in the comment of the player he/she says about organelles ejected which happens in MicrobeDeathSystem but at the same time he/she writes about 'a resource duplication glitch'. It can't be only bonus digestible glucose even if it's 24 like I wrote above. However, it's difficult to evaluate visually how much of it has been ejected after death so a player can think it's the same amount like one digested.

hhyyrylainen commented 3 months ago

I've seen this digested cells drop chunks bug many times myself, and haven't seen the bug with infinite resources. So even just a fix for just literally what the issue title here calls for would be good enough to me.

Also I didn't test that code change I just realized should be made. It is possible that the death is triggered after removing the attached component, meaning that that code change wouldn't be even all that's needed to fix the death logic.

MalyshevArtem commented 3 months ago

@hhyyrylainen Do you remember the system which handles microbe resource clearing after it's been digested? In the EngulfedDigestionSystem there is the HandleDigestion method where a cell becomes digested eventually. At this step the health of the cell is full. I want to see what happens next with the cell. Before it disappears and how it happens.

hhyyrylainen commented 3 months ago

Once the phagocytosis step is set to digested, EngulfingSystem should take over handling the digestion interaction again.

Edit: though looking at the code it is possible that the animation is complete and thus Interpolate is false, meaning that maybe the engulfing system never actually sees the object in digested state. So that could be one bug related to this issue.

MalyshevArtem commented 3 months ago

@hhyyrylainen If the MicrobeDeathSystem is the only one place where a microbe can eject resources after its death, I don't know how it's possible that there is a chance it can die when engulfed.

Yes, the EngulfingSystem finishes digestion process and it places the entity into queuedForDelete in the DestroyEntity. Then the entity of a digested cell appears in the MicrobeDeathSystem but there is the if statement in the HandleMicrobeDeath:

        if (entity.Has<AttachedToEntity>())
        {
            // When in a colony needs to detach
            if (entity.Has<MicrobeColonyMember>())
            {
                commandRecorder = worldSimulation.StartRecordingEntityCommands();

                if (!MicrobeColonyHelpers.UnbindAll(entity, commandRecorder))
                {
                    GD.PrintErr("Failed to unbind microbe from colony on death");
                }
            }
            else
            {
                // Being engulfed handling is in OnKilled and OnExpelledFromEngulfment
                // Dropping corpse chunks won't make sense while inside a cell (being engulfed)
                return true;
            }
        }

At this time it actually has AttachedToEntity so when it goes into then it goes into the else. In this way even if it has values in the CompoundBag and AdditionalEngulfableCompounds it can't eject them. It can only do it if we don't have the else. I don't know how much time ago the game was redesigned to implement ecs and the else was skipped. And when the first complaints were received about dying during digestion.

hhyyrylainen commented 3 months ago

Did you run with that code enabled? That's not what's currently on master branch. If the problem cannot trigger with that code as-written then that code-change I suggested above would fix this problem, or at least make it much less likely.

MalyshevArtem commented 3 months ago

I didn't. I just wanted to know about other possible places in the code where a microbe can eject resources after its death.

As it's the only one, I'm gonna try to catch it somehow in the MicrobeDeathSystem.

hhyyrylainen commented 3 months ago

Well there are 2 usages of SpawnCorpseChunks one in the death system, but interestingly another in Engulfable.OnExpelledFromEngulfment (which is an extension method). That code should only trigger when intentionally ejecting something that is so digested that it should be killed. If I remember right this plays the eject animation before killing the cell that is ejected, which isn't what happens in the situation described by this issue.

MalyshevArtem commented 3 months ago

What I've done so far.

In the if (entity.Has<AttachedToEntity>()) of the HandleMicrobeDeath I just tried to catch microbes with the DigestedAmount equal to 1. I could do it.

Then I just tried to catch microbes in this if with compounds in a cell and additional compounds equal to more than 0. I could do it.

But I can't catch any when these two 'if's in. The game ran for three hours and there wasn't any case.

So it must be very rare if I do it right.

hhyyrylainen commented 2 months ago

Have you tested without your changes to see what's the expected time until this bug triggers? I thought I was able to see this within like 5 minutes in freebuild when I made a basic eukaryote and went around engulfing smaller cells.

MalyshevArtem commented 2 months ago

No, I'm not. I'm going to do it on the weekend. Now I have a lot of work to do.

hhyyrylainen commented 2 months ago

I committed that added return value and a few other smaller tweaks here: https://github.com/Revolutionary-Games/Thrive/pull/5496

I'm not sure if those would make this problem go away or not, but when you continue on this you should pull the latest master to make sure you are testing with those fixes I made in.

MalyshevArtem commented 2 months ago

Ok.