Autodetect float vs integer _minlight

[SirYodaJedi]

Issue

Currently, minlight is hardcoded to be interpreted as a float between 0 and 2 (or higher for BSPX HDR), only if outputting a Q2BSP:

ericw-tools/light/ltface.cc

Lines 624 to 627 in bfea46c

	// Q2 uses a 0-1 range for minlight
	if (bsp->loadversion->game->id == GAME_QUAKE_II) {
	lightsurf.minlight *= 128.f;
	}

This is consistent with QRAD3 (Quake 2) and QRAD (HL1).

However, this makes things a bit inconsistent between EWTs output formats, since the other three output formats expect minlight to be formatted as an integer between 0 and 512 (or higher for BSPX HDR). This sort of inconsistency is a bit confusing, since it's not an actual format difference.

Proposal

Ideally, it should detect if it's an integer or decimal, like with _color: if there's a decimal point, multiply by 128, otherwise leave as-is.

Potential caveats

• This could potentially break something if someone used a decimal point with the 0-512 notation, but that seems unlikely, since the lightmap doesn't have that sort of precision.
• This would break the ability to just say _minlight 2 for full overbright in Q2 mode, but the mapper could just change the 2 to 2.0.

[SirYodaJedi]

Also related to #437, I guess, since vanilla HLBSP compilers expect minlight to be 0-2, but EWT expects 0-512.

[ericwa]

I appreciate the goal, to unify how .map's (or by extension, the light settings in a .bsp entity lump) are interpreted, regardless of output format, and I try to stick to that where possible.

The check for Q2BSP format as a hint to switch scales from 0-512 to 0-1 was the best compromise Paril and I could come up with, when adding Q2 support. We do have it noted in the documentation here: https://ericw-tools.readthedocs.io/en/latest/light.html#worldspawn-key-light

If we used the presence of a decimal point as a way to switch scales, I am concerned about breakage to existing maps, e.g. in the test maps I already have a Q2 test map that uses "_minlight" "1" to mean 1.0 i.e. 100% brightness, that would be broken by this change:

ericw-tools/testmaps/q2_minlight_inherited.map

Line 101 in bfea46c

"_minlight" "1"

Additionally, interpreting 1 and 1.0 on different scales that differ by a factor of 256x could be surprising.

like with _color: if there's a decimal point, multiply by 128, otherwise leave as-is.

What "_color" does, is check if all components are in [0..1]:

ericw-tools/include/common/qvec.hh

Lines 728 to 737 in bfea46c

	/* detect colors with components in 0-1 and scale them to 0-255 */
	template<typename T>
	constexpr qvec<T, 3> normalize_color_format(const qvec<T, 3> &color)
	{
	if (color[0] >= 0 && color[0] <= 1 && color[1] >= 0 && color[1] <= 1 && color[2] >= 0 && color[2] <= 1) {
	return color * 255;
	}
	return color;
	}

This has its own problems, mainly that it's incompatible with HDR colors, but it's probably too late to change (IIRC I added this in ~2015 in the very first versions of these tools).

---

What I would lean towards instead is:

• keep the current approach, extend the Q2 hack to HLBSP - I didn't know that HL tools used 0-1.
• If there's demand, we could make this explicit in a flag, something like -minlight_scale [zero-one|zero-512] or normalized-float|integer etc. - not sure how to name those. So hpothetically, if porting a Q1 map to Q2, you could use -minlight_scale integer to tell it to use the 0..512. Although, I wonder if by the time someone looks up and finds this option, it'd be quicker to just rescale the minlights on everything.

[SirYodaJedi]

Although, I wonder if by the time someone looks up and finds this option, it'd be quicker to just rescale the minlights on everything.

This is probably true; most maps probably only have a handful of minlights at most (compared to say, lights, which are much more tedious to convert).

Also, interesting about how the _color type detection works. I guess that means if someone was weird and really wanted #010101, they'd have to do "0.0039 0.0039 0.0039" instead of "1 1 1".