-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
v0.18.0 introduces major bugs with collisions with sleeping bodies spamming collisionEnd indefinitely #1077
Comments
Hey thanks for reporting, I'll look into this but if you happen to have a minimal example please do share. Is there anything visible other than just that event getting too many calls? |
Unfortunately I don't have a minimal example to share at the moment but if you have any difficulty reproducing the issue I can try to elaborate or generate an example. My implementation is running in Node as the back-end for a Phaser game. As soon as I move the player over a static asset (created for example with I created an implementation level fix in our game by ensuring our static bodies do not unintentionally get converted to sleeping by the global Engine config, by setting |
@liabru if you have any consulting hours to spare we'd be happy to pay an hourly rate for you to review the integration of MatterJS in our game as we're working on performance optimizations for it. We have it set up in docker so pretty easy to get set up locally with it. |
@liabru I have also been seeing this bug the past few days. In my instance I have a game where when a body collides with another particular static body, the first body will be set to sleeping then removed after a set amount of time. What I am noticing is that the collide end event will continue to fire even after the body is removed from the world. I set up a code pen to demonstrate: |
Thanks for the info both and @tropicalpudding for providing that example. I've a preliminary fix for it here if you'd like to try it out and help review, that would be much appreciated: #1079 |
@liabru thanks for the quick patch! I tested it just now and unfortunately am still seeing the collisionEnd event getting triggered indefinitely after a collision exit. I also noticed CPU load for our test scenario, 10,000 completely static sleeping bodies and 1 active body goes from 20-30% load on v17.0.1 to average 40% load with this patch version. |
@fishmongr is it possible the As for using 10,000 bodies it sounds like you may really need to start looking into your own broadphase (it would be a case of swapping out |
@liabru I just retested with some additional logging on Thanks for the feedback on our approach. We are in the early stages of looking at options like that. We were surprised to find that loading 10,000 completely static walls and obstacles in our physics world would sustain such high CPU load (let alone any CPU load at all) when no active player bodies were added to the physics world to interact with them. Is that behavior you would expect? We were assuming our matterJS collision groups configuration was off but it appears to be up to what is outlined in the documentation. |
@fishmongr in the example provided by @tropicalpudding (which the PR seems to solve, but still testing through) the problem is specifically caused by bodies being removed (I think). I've been trying out collision events with sleeping enabled without body removal and can't seem to trigger the end event being called like that so far. Can you try narrow down a test case for what you are seeing? At the moment the default broadphase is much more optimised for dynamic bodies, but statics shouldn't be all that slow unless you're hitting some sort of |
@liabru I created a version of @tropicalpudding example that more matches our use case here: https://codepen.io/fishmongr/pen/mdBQaPP?editors=0011 I found I originally wasn't testing the patch when I thought I was because I was pulling it in via Thanks again for the additional insight on the current broadphase implementation being oriented towards dynamic bodies. I'm hoping we can make some easy tweaks in our own version of Matter.Detector or some small lib modifications in a fork to allow the lib to meet our needs in terms of static body scale. If you have hourly consulting bandwidth available our company would be happy to pay for your time on updating the lib to handle static bodies much more efficiently. This wouldn't need to be anything custom for us, but just improvements to the library itself. Let me know if that is something you'd have the bandwidth and interest in doing at this time. Thanks! |
After digging in it turns out a more complete fixed required a few more edge cases. I've updated the alpha build on #1079 if you want to try it. @fishmongr thanks for providing the additional example it looks like the above update is working on it, take a look if you can and let me know. I'll likely look to improve static scaling later on but for now other improvements have priority unless I see that it's affecting a lot of users (sorry not looking to consult on this at the moment though but appreciate the suggestion). |
@liabru fix is confirmed! Thank you. I have updated the CodePen above that shows the single collisionEnd is dispatching now. No prob on the priorities. If your bandwidth or priorities with the lib change our offer should stand for some time, you can give me a ping at [email protected]. Unfortunately we'll likely need to stick to v0.17.1 for our use case due to the performance impact of v0.18.0 for our use cases. |
Great thanks for checking it out and confirming, I'll get a release out once I've finished testing through. Yeah |
Just thought that I should mention that I got this problem (collisionStart and collisionEnd is spammed) on v0.18.0 myself, but not in regards of sleeping bodies. But when I have a collision between a body in engine.world and bodies placed in a composite in engine.world. If I remove all of the bodies from the composite and add it to the engine.world directly instead, I dont get this bug. But this is probably something that you're already aware of? (I have tested revertig to v0.17.1, and there it works fine) (Also since this this is my first comment on this project: MatterJS is awesome, you are awseome(!!!), and thank you so much for your work on this project 🙌🙌🙌🙌 ) |
@erex1 that sounds slightly different actually but maybe related, would you mind throwing an example of that like ones above? Also you can try the fix alpha build to see if that resolves it already. |
@liabru Ok, I may have jumped to conclusions. Even though adding the items to a composite triggered the bug in my project, it must be something more to it. Because when I try to have a composite and a body in a world in a simple codepen, it works fine. I'll need to do some more testing to find out what other factors that combine to reproduce the issue. Also I will test the alpha build when i get home to my computer 👍 |
@liabru Well this is awkward! I managed to reproduce it. I was adding bodies to the composite in a for loop, but I also managed to add the composite to the world in the for loop and not outside it. So basically if you add the same composite to the world twice it causes the infinite calls to collision start and end. So the weird thing is probably that it works perfectly fine on v0.17.1, because I would guess that adding the same composite twice is not something that should/needs to be supported? Not sure if it is of interest anymore 😅 , but I created this codepen that reproduces this issue (by adding the composite twice): https://codepen.io/erex1/pen/yLzGwJe |
Sounds good, thanks. BTW we forked v0.17.1 and commented out some functionality we didn't need and found something like a 600% improvement in CPU performance with those 10,000 static bodies in the world while our active bodies still worked the same. Could be a good reference for things that could be removed from your internal loops for static and sleeping bodies: aavegotchi@3ca1403 10,000 is just a performance test to understand the boundaries of the physics system. As this engine is very popular with Phaser and Phaser is used to build large MMPORG game worlds I completely agree that the game logic itself should be optimized for the use case and shouldn't completely lean on the physics engine to do that, but the lib does seem to be unusually unoptimized for the static / sleeping elements of larger more serious game projects trying to leverage it. We plan on scaling the engine by splitting our huge maps into separate physics zones but the number of separate physics engine zones required depends most significantly on the performance of the engine with X number of bodies of all types. |
I wanted to report that I has similar issues with sleeping collisions when I switched to 0.18 (the issue for my game occurred when you use an ability that freezes time for everyone but the player called time dilation, https://landgreen.github.io/sidescroller/ ) |
@landgreen thanks for the confirmation, I had been unsure about that fix but that's a great sign if it's working out in your game (nice updates too!) |
* master: (22 commits) improve test comparison report update ci update ci preserve pair.contacts order optimised Resolver.solvePosition bump package lock improve test comparison report fixed compare tool layer order in demo testbed fixed compare tool layer order in demo testbed added multi example testing tool to demo added body removal to Example.remove changed Composte.removeComposite and Composte.removeBody to reset body.sleepCounter optimised Collision.collides fix collision events for sleeping pairs, closes #1077 added local pairs functions in Pairs.update removed pair.confirmedActive changed Pair.id format to use shorter ids optimised Resolver.solveVelocity optimised contacts and supports memory and gc use optimised pairs and collisions memory and gc use ...
Upgraded from v0.17.1 to latest to hopefully see some performance upgrades. Verified performance is pretty much the same but there is a nasty bug introduced if you have Engine
enableSleeping
true. Bodies that collide with sleeping bodies will have Matter.Events.oncollisionEnd
spammed indefinitely until you restart the server. It's pretty easy to reproduce.This issue will likely happen more frequently than expected to anyone using Engine
enableSleeping
true as it's quite difficult to manually manage sleeping bodies when you want and not have the lib auto sleep everything including static bodies, this is due tosleepThreshold=-1
being so difficult to set in a manner that sticks, but that is an unrelated older usability issue.The text was updated successfully, but these errors were encountered: