Skip to content

Implement Score Motor component#435

Merged
freezy merged 51 commits into
masterfrom
feature/score-motor
Sep 15, 2022
Merged

Implement Score Motor component#435
freezy merged 51 commits into
masterfrom
feature/score-motor

Conversation

@jsm174
Copy link
Copy Markdown
Collaborator

@jsm174 jsm174 commented Aug 26, 2022

This PR closes #421.

It adds a new ScoreMotorComponent that can be configured to correctly one or many ScoreReelDisplayComponent.

ScoreReelDisplayComponent has been updated to look for a ScoreMotorComponent. If found, the display will keep track of its score.

The Score Motor can be configured in the inspector:

Screen_Shot_2022-08-26_at_5 29 26_PM

The default settings are suitable for a Gottlieb EM, ie: 6 steps in 120 degrees over 769ms.

The component forces a working configuration, ie, you can not configure less than 5 steps.

The ScoreMotorComponent also exposes two switches Motor Running Switch and Motor Step Switch that can be configured in the Switch Manager:

Screen_Shot_2022-08-26_at_5 28 51_PM

There will be a follow up PR that adds new Visual Scripting Units to tie this all together.

Thanks again to @freezy, @Scottacus64, @bord, and @Cupiii for the several hours of discussion to get this figured out.

@Scottacus64
Copy link
Copy Markdown
Collaborator

This looks awesome! Thanks for all of the hours you put into this!

@jsm174
Copy link
Copy Markdown
Collaborator Author

jsm174 commented Aug 28, 2022

After discussions on Discord, @freezy suggested using the existing Update Display unit instead of an additional Add Points unit.

I've refactored the code accordingly, and it really makes everything a lot more clean. The ScoreReelDisplayComponent now keeps track of its score internally.

I've also added selecting the ScoreMotorComponent directly on the ScoreReelDisplayComponent. This would allow us to have multiple score motors with different configurations for testing:

Screen Shot 2022-08-28 at 2 11 08 PM

If a ScoreMotorComponent is not attached to the ScoreReelDisplayComponent, everything works exactly like it did before.

@jsm174
Copy link
Copy Markdown
Collaborator Author

jsm174 commented Aug 28, 2022

The last remaining issue is to revisit the score reel animation. It can often times get out of sync with the reels internal score.

Copy link
Copy Markdown
Owner

@freezy freezy left a comment

Choose a reason for hiding this comment

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

Thanks, this is so cool!

Added a few comments. One thing I still don't understand is why can't you completely replace the add points methods by set point methods? Then you can also unify it with reset. I still don't get why reset is any different from "set points to 0".

Awesome stuff though. Will check out the UVS part tomorrow.

Comment thread VisualPinball.Unity/VisualPinball.Unity/Display/DisplayComponent.cs Outdated
Comment thread VisualPinball.Unity/VisualPinball.Unity/Display/ScoreReelDisplayComponent.cs Outdated
Comment thread VisualPinball.Unity/VisualPinball.Unity/Display/ScoreReelDisplayComponent.cs Outdated
Comment thread VisualPinball.Unity/VisualPinball.Unity/Game/Engine/IGamelogicEngine.cs Outdated
Comment thread VisualPinball.Unity/VisualPinball.Unity/Game/Engine/IGamelogicEngine.cs Outdated
Comment thread VisualPinball.Unity/VisualPinball.Unity/VPT/Mech/ScoreMotorApi.cs Outdated
Comment thread VisualPinball.Unity/VisualPinball.Unity/VPT/Mech/ScoreMotorApi.cs Outdated
Comment thread VisualPinball.Unity/VisualPinball.Unity/VPT/Mech/ScoreMotorApi.cs Outdated
Comment thread VisualPinball.Unity/VisualPinball.Unity/VPT/Mech/ScoreMotorApi.cs Outdated
@jsm174
Copy link
Copy Markdown
Collaborator Author

jsm174 commented Aug 30, 2022

Added a few comments. One thing I still don't understand is why can't you completely replace the add points methods by set point methods?

The reason why I'm using add points is because of the blocking.

For example, we have 1000 points and we tell the score display to update 500 points:

  • score motor starts running
  • first increase of 100, the internal score is 1100
  • incoming points request for 10 points and blocking disabled, we add 10 points, the internal score is 1110
  • second increase of 100, the internal score is 1210
  • incoming points request for 500 points, and we throw it out
  • incoming points request for 10 points and blocking disabled, we add 10 points, the internal score is 1220
  • third increase of 100, the internal score is 1320
  • fourth increase of 100, the internal score is 1420
  • fifth increase of 100, the internal score is 1520
  • score motor stops running

Then you can also unify it with reset. I still don't get why reset is any different from "set points to 0".

Since we had the Update Display visual scripting unit, and each display offers a clear method, I thought a Clear Display unit would be a good addition.

Reset is doing a little more than just setting the reels to zero. It follows all the same timing rules, and has logic to keep the motor running for an additional turn if needed.

Say the score is 2230, one turn is going to advance the digits 5 times:

3340, 4450, 5560, 6670, 7780

We still need to respect that timing (wait, increase, increase, increase, increase, increase).

Since we are not at zero, the score motor continues for another revolution

8890, 9900, 0, 0, 0

(wait, increase, increase, increase, increase, increase).

@freezy
Copy link
Copy Markdown
Owner

freezy commented Aug 30, 2022

Good point, I didn't think about the concurrency issue. Now it makes sense!

I'll go through it again tonight and merge if all is good.

Thanks for bearing with me :)

@freezy
Copy link
Copy Markdown
Owner

freezy commented Aug 30, 2022

Oh, one more thing.

I suppose that currently, in UVS, it's entirely possible for a user to put scores like 1500. How do we deal with that? Error, warning, or fall back to non-score-motor driven scoring for those cases?

@jsm174
Copy link
Copy Markdown
Collaborator Author

jsm174 commented Aug 30, 2022

Good point, I didn't think about the concurrency issue. Now it makes sense!

I'll go through it again tonight and merge if all is good.

Thanks for bearing with me :)

I went through and cleaned up the DisplayUpdateEvent to just DisplayEvent. Makes it more like SwitchEvent CoilEvent etc.

I also removed DisplayPlayer from Displays. They now use an OnDisplayChanged event.

@jsm174
Copy link
Copy Markdown
Collaborator Author

jsm174 commented Aug 30, 2022

Oh, one more thing.

I suppose that currently, in UVS, it's entirely possible for a user to put scores like 1500. How do we deal with that? Error, warning, or fall back to non-score-motor driven scoring for those cases?

The score motor figures out the increase doesn't make sense and then logs the error, and the points are thrown away.

if (increase > ScoreMotorComponent.MaxIncrease) {
   Logger.Error($"too many increases (ignoring points), id={id}, points={points}, increase={increase}");
   return;
}

Comment thread VisualPinball.Unity/VisualPinball.Unity/VPT/Mech/ScoreMotorComponent.cs Outdated
Copy link
Copy Markdown
Owner

@freezy freezy left a comment

Choose a reason for hiding this comment

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

Okay for me now.

In the future, to do timing-related stuff, coroutines might be an easier approach.

Can be merged once the doc is done.

@freezy
Copy link
Copy Markdown
Owner

freezy commented Sep 2, 2022

Oh, and don't forget the UVS doc for the new nodes!

@freezy
Copy link
Copy Markdown
Owner

freezy commented Sep 2, 2022

Also, PinMAME needs to be updated with the new API (and preferably MPF as well)

@jsm174 jsm174 force-pushed the feature/score-motor branch from 0b62d3e to eeb736a Compare September 5, 2022 23:55
@freezy freezy merged commit 33ed8c9 into master Sep 15, 2022
@freezy freezy deleted the feature/score-motor branch September 15, 2022 06:48
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.

Properly simulate score reels

3 participants