Big endian support for BSP loader #1466
Conversation
a1batross
requested review from
a1batross,
mittorn,
nekonomicon and
SNMetamorph
| @@ -337,6 +337,10 @@ static wfile_t *W_Open( const char *filename, int *error ) | |||
| return NULL; | |||
| } | |||
|
|
|||
| LittleLongSW(header.ident); | |||
There was a problem hiding this comment.
Spaces in parentheses. :)
It's fine to not have them in type casts and between consequent parentheses, but they should be everywhere else:
For example:
(T *)(expr + (expression2 * n))
should be formatted as:
(T *)( expr + ( expression2 * n ))
Sorry, don't have clang-format for this yet, but we probably will migrate to something similar later.
| @@ -303,7 +303,7 @@ model_t *Mod_LoadModel( model_t *mod, qboolean crash ) | |||
| loaded = true; | |||
| break; | |||
| case Q1BSP_VERSION: | |||
| case HLBSP_VERSION: | |||
| case LittleLong(HLBSP_VERSION): | |||
There was a problem hiding this comment.
I suggest you to change this:
uint32_t version;
version = LittleLong( *(uint *)buf );
So you don't have to add macro calls to every version check.
| { | ||
| dheader_t *header = (dheader_t *)mod_base; | ||
| #ifdef XASH_BIG_ENDIAN | ||
| LittleLongSW( header->version ); |
There was a problem hiding this comment.
You're potentially writing to const here. While this memory isn't read-only, I'm not sure if it's a good solution here. I think we can remove const modifier from BSP model loader for now.
Also, you can use static inline instead of _inline. Or even just static, since most compilers do ignore inline keyword.
_inline is a legacy macro definition I forgot about, it's almost not used in new code.
| @@ -1913,7 +1916,15 @@ static void Mod_LoadVertexes( dbspmodel_t *bmod ) | |||
| { | |||
| if( bmod->isworld ) | |||
| AddPointToBounds( in->point, world.mins, world.maxs ); | |||
| #ifdef XASH_BIG_ENDIAN | |||
There was a problem hiding this comment.
You may completely remove this check. Compiler will optimize out a loop that does nothing on little endian.
Also, if you didn't know, platform macros are defined into positive values in build.h, so they can be checked with just #if and used in complex expressions without defined() usage.
| @@ -80,8 +80,9 @@ GNU General Public License for more details. | |||
| #define Q_round( x, y ) (floor( x / y + 0.5f ) * y ) | |||
| #define Q_rint(x) ((x) < 0.0f ? ((int)((x)-0.5f)) : ((int)((x)+0.5f))) | |||
|
|
|||
| #ifdef XASH_IRIX | |||
| #if defined( XASH_IRIX ) && defined( __GNUC__ ) | |||
There was a problem hiding this comment.
Same as above. You can just use simple and readable #if XASH_IRIX && __GNUC__.
|
Also, your RTC battery seems worn out!
Though I don't care about commit date to be honest. But you still better rewrite the commits because the commit messages must be in a specific format: You can easily rewrite commits using |
|
Looks like this guy completely lost somewhere. Maybe we should complete this PR ourselves? It isn't complex nor massive, that shouldn't be a problem. The only problem is we couldn't test it on Irix. So, maybe he'll come back and do it himself, or somebody else who have Irix hardware will do it for us. п.с. а может быть спросить у Бачило, может он бы согласился помочь с этим? :) |
This set of patches seems like it's enough to get the game loading BSPs on big endian platforms. Tested on big endian mips-irix using half life 1 maps. This does not have support for extra lumps and doesn't support the QBSP2 files.
I probably missed something in here because there's quite a few fields in a BSP so please let me know if you notice anything.