Skip to content
This repository was archived by the owner on Nov 28, 2025. It is now read-only.

Conversation

@xskutsu
Copy link

@xskutsu xskutsu commented Jun 22, 2025

This PR does a lot of invasive changes that drastically improve the performance of the game.

The source of this commit is mostly original, however some of it originates from my in-development game Polytanks.

Here is a Before and After image (look at server speed percentage). It's not a perfect demonstration, but you understand.

Before

image

After

image

There is further work that could be done such as reworking how socket views are handled and make them query the grid or reworking that stupid overly-detrimental-to-runtime-performance Proxy-config thing, however I got bored.

@xskutsu
Copy link
Author

xskutsu commented Jun 22, 2025

It's also worth mentioning that there is a good chance many of these changes can be ported to the aps++ rewrite.

Should be pretty easy to port these manually, however if explicitly requested I can make a similar PR for that project.

@Taureon
Copy link
Owner

Taureon commented Jun 22, 2025

Almost all of this looks good to me!
Only worry is that IIFE added to gameDraw.js, cuz I don't see why the caches can't just be put in the top level.

...that stupid overly-detrimental-to-runtime-performance Proxy-config thing...

Fully agreed, I think it'd be better to just make it a hashmap without any of the case-translation and without the change event.

What could work is warning in console for setting dublicate convars.

@xskutsu
Copy link
Author

xskutsu commented Jun 22, 2025

Yeah the color cache could absolutely be taken out of there to remove the IIFE. I did that to keep it isolated from the rest of the code as nothing else should ever need to read it. Also, almost always, IIFEs have no significant impact on performance, especially in cases like this where it's only initializing once. There's even the chance it helps performance by avoiding pollution of the higher scope, though we'd be measuring sub microseconds at that point...

@AE0hello
Copy link
Collaborator

Woa! Thats a massive changing of codes, im very impressed, i will test it soon with the aps++ rewrite.

@xskutsu
Copy link
Author

xskutsu commented Jun 22, 2025

Thank you! If you want any help or have questions about implementation ask me here or at my discord xskt. (DMs open!)

@AE0hello
Copy link
Collaborator

Alright, i only see one problem with the "nearest" function, wich its not working properly on the rewrite.

@Taureon
Copy link
Owner

Taureon commented Jun 23, 2025

any other problems before i merge this?

@AE0hello
Copy link
Collaborator

AE0hello commented Jun 23, 2025

nearestDifferentMaster controller is lagging the server very hard if serveral entities uses nearestDifferentMaster controller, he may look foward to fix it, but otherwise there are no problems.

@xskutsu
Copy link
Author

xskutsu commented Jun 23, 2025

nearestDifferentMaster controller is lagging the server very hard if serveral entities uses nearestDifferentMaster controller, he may look foward to fix it, but otherwise there are no problems.

You're misunderstanding. It's not that nearestDifferentMaster is any laggier than before. (It's actually faster than it was originally.) It's that now, after optimizing the other parts, it has become the slowest thing in comparison.

@AE0hello
Copy link
Collaborator

Oh okay

@xskutsu
Copy link
Author

xskutsu commented Jun 23, 2025

any other problems before i merge this?

As far as I know there is not. Feel free to merge when the code has been reviewed in full.

If there are any new issues discovered later, feel free to contact me on discord about it and I'll try to fix

@NightFuryToothless
Copy link
Collaborator

NightFuryToothless commented Jun 24, 2025

Great pull request! There are some good optimizations.

On case with query() function in hashgrid.js file, which is used in nearestDifferentMaster. Running benchmarks, this function can take up to 95% of all entity controllers running time.

Screenshot from 2025-06-24 12-11-43

Below you can see that I'm benchmarking all controllers, not just nearestDifferentMaster controller for all entities. This is siege, so there are other controllers running as well.

Screenshot from 2025-06-24 12-13-36

@AE0hello
Copy link
Collaborator

Yea and that causes to lag a ton then the original nearestDifferentMaster

@xskutsu
Copy link
Author

xskutsu commented Jun 24, 2025

Again just to clarify, because I'm not sure if you are just pinpointing the lag or claiming there is an issue present, this isn't new lag introduced by the PR. nearestDifferentMaster was actually optimized here. Previously it would iterate over all entities and perform a raw distance check while now it uses the hashgrid query function which is more efficient overall.

What is happening is that after optimizing the other parts of the game, that before lagged even harder than this, nearestDifferentMaster has become the relatively most expensive part of the game loop. Not because it is worse, but because everything else that competed for compute time per tick in the game loop is faster now.

@AE0hello
Copy link
Collaborator

If you say so.

@AE0hello
Copy link
Collaborator

AE0hello commented Jun 24, 2025

I have solved the massive lag on nearestDifferentMaster, appearently i had to change one line of code and that solved it.
This:
image
To this:
image

I would recommend to use the hashgrid for collision checks only, not anything else. Anyway, since that is solved, now it can go up to 1700 entities instead of 700 which was the cap before. Because of that line of code.

@xskutsu
Copy link
Author

xskutsu commented Jun 24, 2025

That literally makes no sense whatsoever. The whole point of a hashgrid is to be faster than O(n) scanning through entities, and even yourself say its faster when its used in collisions. It makes zero sense for it to be slower here, as its the same type of optimization.

@xskutsu
Copy link
Author

xskutsu commented Jun 24, 2025

I'll look into it more later though, maybe the entire controller can be redone in a better way than it was originally.

@AE0hello
Copy link
Collaborator

Not working, they stopped targetting entities

@NightFuryToothless
Copy link
Collaborator

NightFuryToothless commented Jun 25, 2025

It seems to work fine, but all the performance improvements come from cutting the range by half.

const sqrRange = range * range;
const halfrange = range / 2;
const fourThirdsSqrRange = sqrRange * 4 / 3;
let mostDangerous = -1;
let finalTargets = [];

With a for loop that looks like this for (const e of grid.query(x - halfrange, y - halfrange, x + halfrange, y + halfrange).values()) {}. But used to look like this for (const e of grid.query(x - range, y - range, x + range, y + range).values()) {}.

The whole point of a hashgrid is to be faster than O(n) scanning through entities, and even yourself say its faster when its used in collisions

I think with collisions it sorts relatively small sizes compared to ranges which can get really big.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants