Opened 2 years ago

Closed 2 years ago

#13918 closed defect (fixed)

SCUMM: MI1 Playback Adjustment settings hide Original GUI setting

Reported by: dwatteau Owned by: tag2015
Priority: normal Component: Engine: SCUMM
Version: Keywords: game settings
Cc: Game: Monkey Island 1

Description

Current Git 83ff98940d6d2 HEAD on macOS.

MI1 CD has an original_gui option, but it's the only version of Monkey1 where you can't change this setting from our interface (but it's there in the config file), possibly because the mi1_intro_adjustment and mi1_outlook_adjustment settings "hide" it.

enable_enhancements is there, though.

I think this is probably related to this part of dialogs.cpp:

// Game options widgets

// Normally this would be added as a static game settings widget, but I see no
// way to get both the dynamic and the static one, so we have to duplicate it
// here.

GUI::CheckboxWidget *ScummOptionsContainerWidget::createEnhancementsCheckbox(GuiObject *boss, const Common::String &name) {
        return new GUI::CheckboxWidget(boss, name, _("Enable game-specific enhancements"), _("Allow ScummVM to make small enhancements to the game, usually based on other versions of the same game."));
}

i.e. we probably just need to duplicate original_gui there, too.

Problem is that whenever we add a new game setting to the SCUMM engine, it will need to be duplicated there as well for MI1, and Loom (which has a similar playback adjustment setting). So I don't know if that's what we want to do, in terms of code maintenance.

Change History (4)

comment:1 by eriktorbjorn, 2 years ago

For reference, this is the part of gui/editgamedialog.cpp that I think is responsible for allowing either a "static" or a "dynamic" settings widget, but not both at the same time:

	//
	// 2) The engine's game settings (shown only if the engine implements one or there are custom engine options)
	//

	if (metaEnginePlugin) {
		const MetaEngineDetection &metaEngineDetection = metaEnginePlugin->get<MetaEngineDetection>();
		metaEngineDetection.registerDefaultSettings(_domain);
		if (enginePlugin) {
			enginePlugin->get<MetaEngine>().registerDefaultSettings(_domain);
			_engineOptions = enginePlugin->get<MetaEngine>().buildEngineOptionsWidgetDynamic(tab, "GameOptions_Game.Container", _domain);
		}
		if (!_engineOptions)
			_engineOptions = metaEngineDetection.buildEngineOptionsWidgetStatic(tab, "GameOptions_Game.Container", _domain);

		if (_engineOptions) {
			_engineOptions->setParentDialog(this);
		}
	}

comment:2 by tag2015, 2 years ago

This bug was solved in PR 4416, I simply added the missing checkbox to the custom engine options.
Of course the preferred solution would be keep the static options and just add the custom ones, but as eriktorbjorn said it would require some work.

As of now the "shared" scumm settings haven't changed much in years, so that's not much of a problem... but if the granular settings for enhancements get implemented, the issue will need to be addressed.

Maybe change to feature request?

comment:3 by dwatteau, 2 years ago

Thanks, I didn't see that your PR got merged!

I agree that the underlying issue with SCUMM settings is probably more of a feature request then, but the original "MI1 Playback Adjustment settings hide Original GUI setting" bug in itself is fixed, now, so I think it's just clearer to close it.

(I could maybe put that idea in a new feature request, but I don't think it's really useful. It's not an idea that's going to be lost, it's more of an internal technical thing that we're going to see the next time some big change needs to be made in those settings.)

comment:4 by dwatteau, 2 years ago

Owner: set to tag2015
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.