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

KOTOR: HUD Scaling #312

Merged
merged 1 commit into from Jul 14, 2018

Conversation

Projects
None yet
2 participants
@Nostritius
Contributor

Nostritius commented Jul 14, 2018

This PR adds the ability to scale the hud of no proper resolution is found.

@Nostritius Nostritius changed the title from KOTOR: If the resolution is not supported scale the hud accordingly to KOTOR: HUD Scaling Jul 14, 2018

scaleWidgetToLowerMid(getWidget("BTN_CLEARONE2"), wHeight);
}
void HUD::scaleWidgetToUpperLeftEdge(Widget *widget, int wWidth, int wHeight) {

This comment has been minimized.

@DrMcCoy

DrMcCoy Jul 14, 2018

Member

You need to check for widget != 0 in all of these here. It shouldn't crash if the widget is not available. For example, the Xbox version doesn't have a lot of these.

@@ -91,12 +92,17 @@ HUD::HUD(Module &module, Engines::Console *console)
resString += Common::UString::format("%dx%d", it->width, it->height);
}
foundRes = &kResolution[0];
scale = true;
warning("TODO: Add scaling for custom resolutions. Supported resolutions are %s", resString.c_str());

This comment has been minimized.

@DrMcCoy

DrMcCoy Jul 14, 2018

Member

That warning needs to go then too, if you're implementing scaling with this PR

@@ -207,6 +213,138 @@ void HUD::setPartyMember2(Creature *creature) {
setPortrait(3, creature != 0, creature ? creature->getPortrait() : "");
}
void HUD::scaleWidgets(unsigned int wWidth, unsigned int wHeight) {
scaleWidgetToUpperLeftEdge(getWidget("LBL_MAPVIEW"), wWidth, wHeight);

This comment has been minimized.

@DrMcCoy

DrMcCoy Jul 14, 2018

Member

Hmm, a lot of code duplication there. And duplicating the widget names partially.

Maybe instead this big chunk, we should rather have a static array of structs with the names of all widgets, their visibility status from the start (taking over the list on lines 106ff) and their relative position.

Something like

enum Position {
	kPositionUpperLeft,
	kPositionMidLeft,
	...
	kPositionUnknown
};

struct KnownWidget {
	const *name;
	bool visible;
	Position position
};

static const KnownWidget kKnownWidgets[] = {
	{ "LBL_MAPBORDER", true, kPositionUpperLeft },
	{ "LBL_LEVELUP1", false, kPositionUpperLeft },
	...
};

Then in HUD::HUD(), we're iterating over kKnownWidgets, setting them invisible if necessary, and scaling them if necessary and a position is given. Any other property we need there?

@Nostritius Nostritius force-pushed the Nostritius:kotor_scaledhud branch 3 times, most recently from 4953f97 to de3fa21 Jul 14, 2018

@Nostritius

This comment has been minimized.

Contributor

Nostritius commented Jul 14, 2018

I reworked the scaling code as you suggested.

@DrMcCoy

This comment has been minimized.

Member

DrMcCoy commented Jul 14, 2018

That's better, yes. Works great, mostly.

One thing, though: the Xbox GUI is called "mi8x6", not "mipc28x6", so just taking kResolution[0] won't work. You need to take the first available one, not the first of all possible ones.

@Nostritius Nostritius force-pushed the Nostritius:kotor_scaledhud branch 5 times, most recently from c9d0f99 to 9056d32 Jul 14, 2018

@Nostritius

This comment has been minimized.

Contributor

Nostritius commented Jul 14, 2018

The code should now look after mipc28x6 and mi8x6 with a preference to mipc28x6

resString += Common::UString::format("%dx%d", it->width, it->height);
// Search for an existing interface with 800x600 resolution with preference to the pc interface.
if (Common::UString(it->gui) == "mipc28x6" || (Common::UString(it->gui) == "mi8x6" && !foundRes)) {

This comment has been minimized.

@DrMcCoy

DrMcCoy Jul 14, 2018

Member

Why are you comparing the name instead width and height directly?

You can also just break when one is found, instead of checking that foundRes is not assigned yet.

I.e. I think that

if ((it->width == 800) && (it->height == 600)) {
	foundRes = &*it;
	break;
}

would be way clearer.

This comment has been minimized.

@DrMcCoy

DrMcCoy Jul 14, 2018

Member

(In C++11, I'd even say that's a std::find() with a lambda checking width and height, but that's future talk :P)

This comment has been minimized.

@Nostritius

Nostritius Jul 14, 2018

Contributor

Since the pc version of kotor also contains the gui file for the xbox ui, i wanted to prefer the pc ui and the only way of doing this, is by comparing their names. Maybe later you can add a command line switch for forcing the xbox ui. Also i wanted to make sure, that in the case of both being found and supported, the pc version is used since that would be what a normal pc user would expect first and, as far as i know, the pc ui is not contained in the xbox game but the xbox ui is contained in the pc game.

This comment has been minimized.

@DrMcCoy

DrMcCoy Jul 14, 2018

Member

That's why availableRes is a std::set, operator<() only checks the dimensions, and the PC version is checked first. availableRes doesn't contain the xbox GUI file in PC version, even though the file is theoretically available.

This comment has been minimized.

@DrMcCoy

DrMcCoy Jul 14, 2018

Member

(Yes, that's a bit of a hack in either case, I know)

This comment has been minimized.

@Nostritius

Nostritius Jul 14, 2018

Contributor

you're right, i will correct this

@Nostritius Nostritius force-pushed the Nostritius:kotor_scaledhud branch from 9056d32 to a56e2ab Jul 14, 2018

@Nostritius

This comment has been minimized.

Contributor

Nostritius commented Jul 14, 2018

i changed the code now as you suggested.

@DrMcCoy DrMcCoy merged commit a56e2ab into xoreos:master Jul 14, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@DrMcCoy

This comment has been minimized.

Member

DrMcCoy commented Jul 14, 2018

Merged, thanks! :)

Can you move that over to KotOR2, too, please?

@Nostritius Nostritius deleted the Nostritius:kotor_scaledhud branch Jul 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment