Skip to content
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

Conveyor optimisations #33870

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

metalgearsloth
Copy link
Contributor

@metalgearsloth metalgearsloth commented Dec 15, 2024

  • Optimise movement for moving stuff. Much better lookup flags + less resolves + slapped parallelrobustjob on it.
  • Avoids tracking contacts on its own.

Needs space-wizards/RobustToolbox#5560 + space-wizards/RobustToolbox#5440

Needs space-wizards/RobustToolbox#5637

I want to give it a shot using shapecasts which will also be a prototype for new mob movement similar to how most games handle it (outside of physics).

Current branch still needs subscribing to airtightchanges or whatever to know if we need to start re-conveying entities.

Resolves #33861

- Optimise movement for moving stuff. Better flags + less resolves + slapped parallelrobustjob on it.
- Sleeping for entities getting conveyed into walls.
@github-actions github-actions bot added S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. size/L Denotes a PR that changes 1000-4999 lines. labels Dec 15, 2024
@metalgearsloth metalgearsloth marked this pull request as ready for review December 21, 2024 22:50
@ScarKy0 ScarKy0 added P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention. T: Performance Type: Performance impacting changes or bugs D2: Medium Difficulty: A good amount of codebase knowledge required. S: Needs Review Status: Requires additional reviews before being fully accepted A: Core Tech Area: Underlying core tech for the game and the Github repository. S: Needs Engine PR Merged Status: Requires an existing Robust Toolbox PR to be merged first. and removed S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Dec 24, 2024
@github-actions github-actions bot added size/M Denotes a PR that changes 100-999 lines. and removed size/L Denotes a PR that changes 1000-4999 lines. labels Jan 10, 2025
@ArtisticRoomba ArtisticRoomba removed the S: Needs Engine PR Merged Status: Requires an existing Robust Toolbox PR to be merged first. label Jan 27, 2025
@github-actions github-actions bot added Changes: Map Changes: Might require knowledge of mapping. size/L Denotes a PR that changes 1000-4999 lines. and removed size/M Denotes a PR that changes 100-999 lines. labels Feb 2, 2025
This reverts commit 1b93fda.
};

var transform = PhysicsSystem.GetLocalPhysicsTransform(entity.Owner, xform);
transform.Position += direction;
Copy link
Member

@slarticodefast slarticodefast Feb 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally the actual movement would be done by offsetting the velocity and letting the physics system handle the movement instead of teleporting it in very small steps. Because currently if an entity is being conveyed it has a velocity of zero even though it is moving, which might cause problems for other systems which need that info (for example my homing projectile PR I'm working on).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a body's InAir ideally it doesn't get conveyed at all (which I imagine a homing projectile would be).

The issue is mob movement entirely dictates the mob's velocity so those systems would need rewriting to handle other input sources.

Copy link
Member

@slarticodefast slarticodefast Feb 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is not the projectile (which does not get conveyed), but the velocity of a target object on a conveyor. That relative motion is important for the homing algorithm to be able to intercept.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt I can cook something up anytime soon as most resources I found on it just adjust position directly because it handles corners better + facilitates not tampering with the CC.

It's just a matter of do we think fixing conveyors tanking the server is more worthwhile than waiting for someone to rewrite conveyor movement as they would have to look at something like velocity sources (which box2d doesn't have innately).

@@ -72,7 +72,7 @@ entities:
- type: MetaData
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this file supposed to be in here?

Copy link
Contributor

github-actions bot commented Feb 8, 2025

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Feb 8, 2025
@github-actions github-actions bot added size/M Denotes a PR that changes 100-999 lines. and removed size/L Denotes a PR that changes 1000-4999 lines. S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted labels Feb 9, 2025
@Emisse
Copy link
Contributor

Emisse commented Feb 10, 2025

what is the map change for

@metalgearsloth
Copy link
Contributor Author

what is the map change for

Conveyor comp coz something getting conveyed, I can prolly make it not saveable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: Core Tech Area: Underlying core tech for the game and the Github repository. Changes: Map Changes: Might require knowledge of mapping. D2: Medium Difficulty: A good amount of codebase knowledge required. P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention. S: Needs Review Status: Requires additional reviews before being fully accepted size/M Denotes a PR that changes 100-999 lines. T: Performance Type: Performance impacting changes or bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Conveyors cause huge lag if there's a ton of items stuck on them
5 participants