ericw-tools icon indicating copy to clipboard operation
ericw-tools copied to clipboard

Autodetect float vs integer _minlight

Open SirYodaJedi opened this issue 3 months ago • 3 comments

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 2 for full overbright in Q2 mode, but the mapper could just change the 2 to 2.0.

SirYodaJedi avatar Nov 30 '25 21:11 SirYodaJedi

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

SirYodaJedi avatar Nov 30 '25 21:11 SirYodaJedi

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] 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.

ericwa avatar Dec 01 '25 04:12 ericwa

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".

SirYodaJedi avatar Dec 01 '25 05:12 SirYodaJedi