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

Some Fixes #72

Closed
wants to merge 0 commits into from
Closed

Some Fixes #72

wants to merge 0 commits into from

Conversation

IParallel
Copy link

I noticed that entity positions where abnormal and i found out the problem was a typo where we were setting the position to the actual delta change

@IParallel IParallel changed the title Entity Position Fix Some Fixes Aug 11, 2024
@@ -450,7 +486,10 @@ private Task HandleOpenWindow(OpenWindowPacket packet)

windowInfo = windowInfo with { Title = packet.WindowTitle };
Logger.Debug($"Received Open Window Packet id={packet.WindowId}");
OpenWindow(packet.WindowId, windowInfo);
Task.Delay(100).ContinueWith(t =>
Copy link
Owner

Choose a reason for hiding this comment

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

Is there a reason why we wait 100ms before opening the window?

Copy link
Author

Choose a reason for hiding this comment

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

i think i sent a video on the discord addressing that temporal change you may want to watch it, i still didn't found the reason of why the task never executes and always times out, but adding a delay make it works.

Copy link
Owner

Choose a reason for hiding this comment

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

hm seems to be a race condition. PR #71 addresses a lot of bugs regarding race conditions, maybe it fixes it.

@Klotzi111
Copy link
Contributor

With entity position there was also a problem where the SetVelocityPacket set the velocity of the entity as their position and thus entities often have almost zero coordinates.
I fixed those problems in #71. You should try it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants