-
Notifications
You must be signed in to change notification settings - Fork 63
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
Rewrite bumpers #483
Rewrite bumpers #483
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looking good!
@toxieainc @vbousquet you guys think that would be worth porting back to VPX?
Hmmm.. How likely is this to break existing VPX tables? Cause otherwise we would have to have 2 codepaths.. |
Well this sounds more like a bugfix to me, no? Do you think there are workarounds in VBS that would break? |
It looks like a nice enhancement but it will likely break existing tables since ball behavior will be changed in bumper area and existing table expects the bug to be present (they have been tweaked for it, or at least people are used to it and expect this not to change). Still the proposed switch area / socket collision / coil collision design look nicer than what we have now in VPX. This could be pushed in for 10.8.1 with 2 codepaths (again...). I think there are other parts that would benefit from a physics refresh (targets, but also flippers since we should be able to do sinced solenoid simulation at some point). We could have a physic switch for these. Anyway, there are perhaps too many things ongoing for a breaking change like this straight now. What do you think ? |
Look like the merge was a bit premature. Tested this and noticed:
I'm working on another branch right now ( |
} else { | ||
insideOfs.SetOutsideOf(collHeader.ItemId, ball.Id); | ||
events.Enqueue(new EventData(EventId.HitEventsUnhit, collHeader.ItemId, ball.Id, true)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't you do the stuff you're doing in BumperApi
in here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could, but then we would be back to hard-coded bumpers that always push the ball away regardless of what the game logic engine has to say about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hang on, that's the goal of the bumpers, no? That's even the case with real-world bumpers if I'm not mistaken. What's a use case where you wouldn't want to do that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you do almost always want the bumper switch to trigger the bumper coil. But I understand the fundamental design of VPE like this: A table is just a bunch of mindless switches and coils, and the game logic engine decides what coils should fire based on the state of the switches. The changes I made align the bumpers with that design. I guess it's a bit pedantic and rarely comes into play. But for example, MPF turns off the bumpers in attract mode and triggers the bumper coil regardless of the switch state in ball search mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, gotcha. I'll need some more time to go through this. I've pulled your latest two PRs and with ring speed 1 the animation looks like this on my branch (but I can't easily test how it was before):
bumper-v1.mp4
So here the ball should approach the center of the bumper much further than right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree it doesn't look quite right. The ring speed is probably not the same as it was before, so that might need some tuning to get right. I haven't changed the size of the collider though. So the issue of how close the ball gets to the bumper is not really related to this PR. Regardless, you can change the radius of both colliders in BumperApi.CreateColliders
. But there are a couple of additional issues here:
- Proportions: The bumper in the video is way too tall compared to the size of the ball. Look at this video: https://www.youtube.com/watch?v=-9JFBaDKxSc The bumper ring is barely above the ball in the starting position. In your video, it's two ball heights above the ground.
- Logic: The ball is redirected when it enters an invisible trigger collider. If you want it to look totally realistic in slow-motion regardless of parameters like bumper height and ball size, you'd probably have to add another collider to the ring and move that down together with the ring, then apply an impulse to the ball the moment it intersects. But this is not worth the effort in my opinion since the difference wouldn't be noticeable at full game speed or from the typical top-down perspective.
I can fix the missing animation. I will try to move the collision into the physics loop as well. Did you wire the bumper switch to the coil? You need to do that now for the bumper to work. |
No, I didn't wire anything. What's the advantage of having to wire this manually? |
Not much of a point in forcing people to wire bumpers manually, I agree. I just didn't think about the usability problem this creates. People will put down a bumper and wonder why it doesn't work. Should probably just add some code that creates a wire whenever a bumper is added to the playfield in the editor. |
Yes, I agree. Maybe my problem was that I didn't do the wiring. I'm writing some doc right now about the changes I'll make to the physics engine in general. If you could resolve the wiring issue and make your additional code burstable, that'd be great, then I'll pull those changes. |
I moved the bumper impulse into the physics loop in #491. I don't know what you mean by this:
|
I've noticed a major problem with this PR. Waiting for the game logic engine or hardware rules to trigger the bumper coil when the bumper is hit makes the interaction between the ball and the bumper frame rate dependent. The game logic can only respond to what happens in the physics engine once per Unity frame, but the game logic engine simulates 1000 steps per second. This causes a few issues with the new bumper code:
All of this combined means that there is a possibility, depending on the speed of the ball, the frame rate, and random chance, for the bumper to 'miss' a ball if it hits the bumper at a high enough speed, because it will bounce off the inner rigid collider out of the outer trigger collider before the game logic can react. And sure enough, here it is: bumper_bug.webm |
I see. Sorry for my slow responses, I'm still working on the transformation refactoring. If it's okay for you, I propose the following: I'll revert this PR on my branch for now, and when I'm done with the transformation changes (hopefully by the end of year), I'll get them back in, similar to how dynamic flippers work. We'd basically auto-activate them, and keep the triggering in the physics loop. What do you think? |
I don't know how dynamic wires should help here. Anything that is outside the physics update job loop will not be fast enough. I added a fix to #491 that tells the collide function in the physics loop whether a wire between the bumper switch and the coil exists. Still don't really like it though. I described why in a comment over there. I'm open to other ideas, I just don't think wires will do the trick. |
Fixes #482
Bumpers now have two colliders:
These balls are pushed away in
IApiCoil.OnCoil
.BallCollider.Collide3DWall
is also called there to make sure the velocity of the ball towards the bumper is removed regardless of how weak the push force of the bumper is. I'm not sure if it's appropriate to do that outside the physics loop, but without it, the pushes of two bumpers (e.g. in a bumper cluster) would cancel each other out like this:output.webm
This is the same test scenario with collision:
fixed.webm
A simpler solution would to simply zero out the ball's velocity, but that would lead to strange interactions when the ball grazes the bumper from the side.