ec- / baseq3a

Unofficial Quake III Arena gamecode patch
26 stars 24 forks source link

Remove BoxOnPlaneSide inline assembly version function and their unused slow version #27

Open LegendaryGuard opened 1 year ago

LegendaryGuard commented 1 year ago

On BoxOnPlaneSide function, that inline assembly code can only be compiled on MSVC compiler (Microsoft Visual C++). That has been tested on win32-msvc solution.

Previously, the code had conditional compilation directives specific to LCC, C_ONLY, id386 and __VECTORC. The refactored code provides an unified implementation that is compatible with multiple compilers.

Additionally, the commit includes a comment explaining the reason for the code change and its relationship to the Kenny Edition and also removes the commented BoxOnPlaneSide2 function being the slow version which is unoptimized.

ec- commented 1 year ago

This function is not used in mod code

LegendaryGuard commented 1 year ago

You're right. This function has never been used in most gamecode mods. But it always has been there and in all Q3 gamecode repositories.

One year ago, in one of my mods, I've experienced an error when I tried to compile with make to get DLLs, the macro conditional blocks of this function weren't correctly handled. So, I researched what it was, the cause was because of this inline assembly code hadn't a macro conditional block that only can be compiled with MSVC.

The other one without inline assembly (which uses C), it's optimized for the most compilers (non MSVC).

ec- commented 1 year ago

Essentially, this is a dead code from mod perspective, so the only correct solution - remove it from mod codebase, otherwise it is totally useless, there is no other 'but...' cases except increasing entropy of this repo

LegendaryGuard commented 1 year ago

Wait. It cannot be removed, otherwise the compiled build in-game won't work and could get some errors when playing. Because it needs to stablish the relationship to the engine. Moreover, this isn't the only function that isn't being used on gamecode, also there are others like ClearBounds, SetPlaneSignbits, Q_rsqrt, ... That's why:

I guess I'll have to open a similar Pull Request on Quake3e about the optimization of this function too.

EDIT: Gotta remove the #if block where ignores if it's not defined Linux and BSD. It doesn't make sense.

LegendaryGuard commented 1 year ago

Ok, when you agree and are ready to merge, squash commits before merging.

LegendaryGuard commented 1 year ago

Done, now the function is the same as Quake3e engine one (the enhanced). I hadn't checked it. Thanks for noticing that detail.