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

Decouple CMS and OPL emulations from dosbox-staging #5220

Closed
wants to merge 10 commits into from

Conversation

RNMB15
Copy link
Contributor

@RNMB15 RNMB15 commented Sep 22, 2024

Add a summary of the change(s) brought by this PR here.

What issue(s) does this PR address?

If there are any related issues, reference them here. Ex. #1234, Fixes #1234, Closes #1234, etc.

Does this PR introduce new feature(s)?

If yes, describe the feature(s) introduced.

Does this PR introduce any breaking change(s)?

If yes, describe the breaking change(s) in detail.

Additional information

Add any additional information here.

@RNMB15
Copy link
Contributor Author

RNMB15 commented Sep 22, 2024

Decouple CMS and OPL emulations from dosbox-staging.

japsmits/dosbox-staging@bc7d5fd

src/dosbox.cpp Outdated
@@ -1393,6 +1393,7 @@ void DOSBOX_SetupConfigSections(void) {
const char* gustypes[] = { "classic", "classic37", "max", "interwave", nullptr };
const char* sbtypes[] = { "sb1", "sb2", "sbpro1", "sbpro2", "sb16", "sb16vibra", "gb", "ess688", "ess1688", "reveal_sc400", "none", nullptr };
const char* oplmodes[]={ "auto", "cms", "opl2", "dualopl2", "opl3", "opl3gold", "none", "hardware", "hardwaregb", "esfm", nullptr };
const char* Set_CMS[]"on", "off", "auto"});
Copy link
Contributor

@maron2000 maron2000 Sep 22, 2024

Choose a reason for hiding this comment

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

const char* Set_CMS[]={"on", "off", "auto"};

@RNMB15 RNMB15 mentioned this pull request Sep 23, 2024
@RNMB15 RNMB15 changed the title Patch 6 Decouple CMS and OPL emulations from dosbox-staging Sep 23, 2024
@Allofich
Copy link
Contributor

To make it like the DosBox Staging commit, in src/hardware/sblaster.cpp you need to change:

line 3721
from
else if (!strcasecmp(omode,"cms")) opl_mode=OPL_cms;
to
else if (!strcasecmp(omode,"cms")) {} // Skip for backward compatibility with existing configurations

line 3736
from
opl_mode=OPL_cms;
to
opl_mode=OPL_none;

In struct SB_INFO add
bool cms;

Somewhere, such as right after the switch (oplmode) at line 3975 ends (at line 4005), add:

		sb.cms = is_cms_enabled();
		if (cms) {
			CMS_Init(section);
		}

And in the destructor ~SBLASTER(), immediately after the switch there add

		if (sb.cms) {
			CMS_ShutDown(m_configuration);
		}

I haven't tested these changes. Currently this PR is not compiling, I don't know if the above changes will fix that.

Also, DOSBox-X has an additional isCMSpassthrough variable. I'm not sure if that needs to be accounted for somehow or can be ignored.

@Allofich
Copy link
Contributor

Allofich commented Sep 25, 2024

You put things within switch statements. When I wrote things like "immediately after the switch," I didn't mean immediately after the line with "switch" in it, I meant after the closing brace of the switch statement. Also you put bool cms; in a switch statement, instead of in struct SB_INFO.

Also I guess you are making your changes through the GitHub editor, which produces "Update xxx" commit messages, but descriptive commit messages are preferable. You can make those with a program like Git Bash, although learning to use it can take some time.

Anyway, I think it would be better to let me make a new PR with the changes. I'll try to do it soon.

@joncampbell123
Copy link
Owner

It looks like CI failed. Is this pull request separate from the other one about CMS and OPL?

@joncampbell123
Copy link
Owner

Should I close this one and merge the newer pull request? At a glance they do basically the same thing, except that this failed the build test.

@Allofich
Copy link
Contributor

Yes, you can close this one.

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.

4 participants