Autodetect float vs integer _minlight
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: https://github.com/ericwa/ericw-tools/blob/bfea46cdbd9dbb2218282a2b8006d1f608dac280/light/ltface.cc#L624-L627 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 2for full overbright in Q2 mode, but the mapper could just change the 2 to 2.0.
Also related to #437, I guess, since vanilla HLBSP compilers expect minlight to be 0-2, but EWT expects 0-512.
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:
https://github.com/ericwa/ericw-tools/blob/bfea46cdbd9dbb2218282a2b8006d1f608dac280/testmaps/q2_minlight_inherited.map#L101
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]:
https://github.com/ericwa/ericw-tools/blob/bfea46cdbd9dbb2218282a2b8006d1f608dac280/include/common/qvec.hh#L728-L737
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]ornormalized-float|integeretc. - not sure how to name those. So hpothetically, if porting a Q1 map to Q2, you could use-minlight_scale integerto 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.
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".