Fix pickup combining over the maximum stack size.

[Howaner]

Fix #1114

[madmaxoft]

I think we can do better.

If you throw 3 stacks of 33 stone each next to each other, this code will fail to combine any of them; we could make code that combines that into (33 + 31) + (2 + 33).

[madmaxoft]

I think we can do better.

If you throw 3 stacks of 33 stone each next to each other, this code will fail to combine any of them; we could make code that combines that into (33 + 31) + (2 + 33).

[Howaner]

done

[Howaner]

done

[madmaxoft]

Ouch, this has a tiny little bug that I had not seen coming.

Consider two pickups close to each other, A has 64 stone, B has 1 stone. The code will first try to move B's 1 stone into A's 64 stone, won't work, nothing will be done. But then it will do the callback in the reverse direction - trying to move A's 64 stone into B's 1 stone, which will work partially by moving 63 stone from A to B. Now A has 1 stone and B has 64 stone. In the next tick, the situation repeats with the pickups reversed. So this makes the server send an update packet for both entities on each world tick :(

Either we solve by checking the pickup ID and moving only from a larger UniqueID to a smaller UniqueID, or we decide that the first solution is enough for our needs. Which will it be?

[madmaxoft]

Ouch, this has a tiny little bug that I had not seen coming.

Consider two pickups close to each other, A has 64 stone, B has 1 stone. The code will first try to move B's 1 stone into A's 64 stone, won't work, nothing will be done. But then it will do the callback in the reverse direction - trying to move A's 64 stone into B's 1 stone, which will work partially by moving 63 stone from A to B. Now A has 1 stone and B has 64 stone. In the next tick, the situation repeats with the pickups reversed. So this makes the server send an update packet for both entities on each world tick :(

Either we solve by checking the pickup ID and moving only from a larger UniqueID to a smaller UniqueID, or we decide that the first solution is enough for our needs. Which will it be?

[madmaxoft]

@worktycho Travis seems to have encountered an interesting failure, care to have a look?

[madmaxoft]

@worktycho Travis seems to have encountered an interesting failure, care to have a look?

[NiLSPACE]

Wouldn't it work if you check if pickup A or B is full?

[NiLSPACE]

Wouldn't it work if you check if pickup A or B is full?

[Howaner]

done

[Howaner]

done

[worktycho]

@madmaxoft Its the copyblocks test. If it doesn't complete in under 10 minutes Travis thinks the build has stalled. Possibly we should reduce its length a little.

[worktycho]

@madmaxoft Its the copyblocks test. If it doesn't complete in under 10 minutes Travis thinks the build has stalled. Possibly we should reduce its length a little.

[tigerw]

Wouldn't it work if you check if pickup A or B is full?

I agree.

[tigerw]

Wouldn't it work if you check if pickup A or B is full?

I agree.

[madmaxoft]

You're right, checking if the pickup was full would work, too. However, we're already checking the UniqueID, so unlike the fullness check, the ID check doesn't cost us anything at all.

[madmaxoft]

You're right, checking if the pickup was full would work, too. However, we're already checking the UniqueID, so unlike the fullness check, the ID check doesn't cost us anything at all.

[Howaner]

done

[Howaner]

done