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

Add game name to browser stream #200

Merged
merged 3 commits into from Dec 4, 2019
Merged

Add game name to browser stream #200

merged 3 commits into from Dec 4, 2019

Conversation

vbousquet
Copy link
Contributor

Add game name (rom) on init request to the browser stream, to allow the requester to adapt its rendering to the game.

The browser stream output allow to create fancy DMD rendering. To allow skinning based on the game rom, you need to get this information from PinMame. The most clean way is for dmdext to pull it on init request, hence this small pull request. This allows little software like DMDOverlay to be more clean and simple.

Add game name (rom) on init request to the browser stream, to allow the requester to adapt its rendering to the game.
@vbousquet
Copy link
Contributor Author

vbousquet commented Dec 2, 2019

To elaborate a bit more, I'm not the author of DMD overlay but like the idea. Here are examples of what you can get with this type of approach (DMD rendering, adapted to each pinball for DMD, glass and frame) ;

Fish Tales
Black Rose
Batman

This allows for 2 screens setup to add an overlay on the backglass, keeping the right aspect ratio for the backglass, and cleanly integrating the whole. All of this, without bloating dmdext with each of the dmd rendering flavour we could imagine.

@vbousquet
Copy link
Contributor Author

And to be complete, I have put the example code (MIT license) for above rendering in a gist here.

@freezy
Copy link
Owner

freezy commented Dec 2, 2019

Cool, I didn't know people were actually using the stream. That's awesome! :)

Could you reformat your changes so the code stays coherent with the rest (mainly replace spaces with tabs)? Then I'll review it!

Thanks!

@vbousquet
Copy link
Contributor Author

New commit fixes the formatting as well as the issue #201

Copy link
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 a lot! Just one small change in the constructor, and we're good to go!

@@ -339,7 +339,7 @@ private void SetupGraphs()
renderers.Add(new VpdbStream { EndPoint = _config.VpdbStream.EndPoint });
}
if (_config.BrowserStream.Enabled) {
renderers.Add(new BrowserStream(_config.BrowserStream.Port));
renderers.Add(new BrowserStream(_gameName, _config.BrowserStream.Port));
Copy link
Owner

Choose a reason for hiding this comment

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

I would prefer this as a second, optional parameter. Reason being that while the browser stream is currently only used for DmdDevice, we might also use it for the dmdext executable, where the game name is not available.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Request integrated in latest commit

Swap the order of parameters for BrowserStream, and made it optional.
Made size message logging more clear.
@vbousquet vbousquet requested a review from freezy December 4, 2019 20:09
Copy link
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.

Cheers, man! 👍

@freezy freezy merged commit f130479 into freezy:master Dec 4, 2019
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.

None yet

2 participants