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

Green Lifter (Claw Guys) Faster in DOS than in Rebirth #247

Closed
Mako88 opened this issue Oct 15, 2016 · 13 comments
Closed

Green Lifter (Claw Guys) Faster in DOS than in Rebirth #247

Mako88 opened this issue Oct 15, 2016 · 13 comments

Comments

@Mako88
Copy link
Collaborator

Mako88 commented Oct 15, 2016

I tested this on D1 level 4 on Hotshot. In the original DOS flying backwards away from one of these bots without any sliding is BARELY enough to get away from them. In Rebirth, it's very easy to pull away while only using reverse with no sliding.

@vLKp
Copy link
Contributor

vLKp commented Oct 15, 2016

Was this correct in 0.58.1-d1x?

@Mako88
Copy link
Collaborator Author

Mako88 commented Oct 15, 2016

Yes, it was correct.

@Mako88
Copy link
Collaborator Author

Mako88 commented Oct 15, 2016

I've realized this is also true of other robots as well. Either the ship is moving faster, or the robots are moving slower. (This seems to be the case across all difficulty levels, but is the most apparent on Hotshot and above).

@Mako88
Copy link
Collaborator Author

Mako88 commented Oct 15, 2016

This happened sometime after commit 93888b4 (A build made at that commit is still correct). The more I compare, the more I don't think it's so much a max speed issue as much as a behavior issue. In older builds (and the original) they bots would constantly chase you, whereas now they seem to randomly slow down or become...disinterested.

@vLKp
Copy link
Contributor

vLKp commented Oct 15, 2016

That is good news. That commit is only 1230 commits back from tip. Since our history is mostly linear, git bisect should be able to find the first bad commit in ~11 steps.

@Mako88
Copy link
Collaborator Author

Mako88 commented Oct 16, 2016

According to git bisect, e6887bb is the first bad commit. (Although that could be slightly off since this is a relatively difficult thing to test accurately).

@vLKp
Copy link
Contributor

vLKp commented Oct 16, 2016

That is probably not the first bad commit. It should have no effect on the generated code, since it is just the choice between passing a pointer and a reference. However, that gives a good starting point for further analysis.

@Mako88
Copy link
Collaborator Author

Mako88 commented Oct 17, 2016

I tried to do another git bisect, but I got a commit that was over 1,000 commits away from the one I posted above (e40172f). This seems like it's going to be tough to debug... :/

@Mako88
Copy link
Collaborator Author

Mako88 commented Oct 17, 2016

For completeness sake, here's how I'm testing:

I fly into the main room on level 4 of D1 on insane. I then get a claw guy a ship length or so away from me where I can see him and fly backwards using only reverse, while turning in a circle and keeping away from the walls (this is about as difficult as it sounds :P ). In 0.58.1 I can do this as long as I want...I'll very VERY slowly creep further away from him, but he'll constantly chase me. In the latest code he'll randomly break to the inside of the circle and stop chasing me for a moment, making it much easier to escape.

Also on Hotshot it seems as if their max speed is less in the current build than in 0.58.1.

@Mako88
Copy link
Collaborator Author

Mako88 commented Oct 19, 2016

Haha! I found the commit: 2a19da8

I discovered it was MUCH easier to see the bug on Hotshot rather than on Insane, and this is the commit I got. It's actually from the same batch of commits as my first bisect (which was done with a much less reliable way of determining it), so I'm confident this is it.

vLKp added a commit that referenced this issue Oct 20, 2016
Mako88 reports that 2a19da8 changed
robot movement.  That commit reordered the operations in a way that
tends to truncate small values prematurely.  Restore the original order
of operations.

Reported-by: Mako88 <#247>
Fixes: 2a19da8 ("Pass object_base &to move_towards_vector")
@vLKp
Copy link
Contributor

vLKp commented Oct 20, 2016

That fits. That commit accidentally changed the order of operations. Difficulty levels below Insane are affected more severely.

@Mako88
Copy link
Collaborator Author

Mako88 commented Oct 21, 2016

Confirmed fixed. Closing.

@Mako88 Mako88 closed this as completed Oct 21, 2016
@vLKp
Copy link
Contributor

vLKp commented Oct 22, 2016

Based on how the values were handled, Ace (difficulty=3) would work correctly. Insane would be close, but not right. Lower difficulties would be more wrong, with Hotshot being the most affected.

In the good build, we compute result = (int)((value * (difficulty + 5)) / 4). In the bad build, we compute result = value * (int)((difficulty + 5) / 4).

For Hotshot (difficulty=2):
Good: (int)((value * 7) / 4) = (int)(value * 1.75)
Bad: (int)(value * (int)1.75) = (int)(value * 1)

For Ace (difficulty=3):
Good: (int)((value * 8) / 4) = (int)(value * 2)
Bad: (int)(value * (int)2.00) = (int)(value * 2)

For Insane (difficulty=4):
Good: (int)((value * 9) / 4) = (int)(value * 2.25)
Bad: (int)(value * (int)2.25) = (int)(value * 2)

That is why it seemed more difficult to spot when using Insane.

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

No branches or pull requests

2 participants