Some 64bit improvements, lots of 64bit TODOs/STUBBED()

[DanielGibson]

This should fix some more 64bit issues.

I also added some more FIXMEs and STUBBED("64-but issue") where the compiler gave me corresponding warnings.

I was able to play the complete first level of the second encounter, however I think there's still a lot missing.

Does anyone know what CShell::GetINDEX() is about? Its return value often is casted to pointer and dereferenced directly afterwards (e.g. in Engine/Network/Network.cpp CacheShadows(); I didn't mark all occurences in the code). Is there some kind of trick (like in ModelPolygonVertex::Read_t() where the "pointers" are actually indexes that are later converted back to pointers) that I'm missing? I'm surprised that INDEX is used to store a pointer there, not ULONG like in several other places..

[yamgent]

I am not officially with Croteam so you need to take this with a grain of salt.

I don't think that it is some kind of indices trick that you mentioned, but rather it is just as what you read in the code: it is merely storing a pointer to the CWorld object.

While the CShell class is typically used for storing console shell variables (in which only some of these variables can be accessed by the user to do stuff such as cheating), it also serves as a way for the game to pass data around different classes, so it conveniently acts like a list of global variables.

Since INDEX and ULONG are of "similar" types (i.e. uses 32-bits in Windows 32-bits), I don't think they would bother with implementing another function just to deal with ULONG, and just cheat by using the INDEX version instead. Of course, that's going to cause problems with your 64-bit goal here ;).

[icculus]

Yeah, INDEX is used for several things in CShell (it's an integer for numeric cvars, all the boolean values are INDEX, too (0 or 1), and occasionally they use it to store pointers cast to integer).

I suspect some pointers ended up in there because it made it easy to pass arbitrary data to shared libraries? Just a guess.

[icculus]

Also, getting a CRC of a pointer is asking for trouble; where does it end up needing that?

[DanielGibson]

The CRC of a pointer is used in Engine/Graphics/DrawPort.cpp CDrawPort::GetID()

[DanielGibson]

So, what should we do about CShell::GetINDEX()? introduce a CShell::GetIntPtr() (and all the other code to make that work)?
or just make GetINDEX() return (and internally use etc) size_t (or probably ssize_t)?

[DanielGibson]

ok, I /think/ GetINDEX() is only used as a pointer for "pwoCurrentWorld", so the cases should be easy to find, and maybe we could even special case them with something like void CShell::SetCurrentWorld(CWorld *pwo) and CWorld* CShell:GetCurrentWorld(void) instead of adding an additional ShellTypeType etc

I haven't even tried to understand the serious sam cvar system yet, could the easy way with GetCurrentWorld() work or would everything break apart if currentworld is not in a named console variable?

[icculus]

I wouldn't add a pointer type to CShell...I think mostly these are meant to be persistent vars that live in config files (and maybe get sent over the network?), so there's really no place it's safe to be passing pointers around in there. If it's really just one place doing it, let's just have a non-CShell solution (even just a global variable, if that makes sense to do. CShell::SetCurrentWorld() might even be overkill if this doesn't otherwise have anything to do with CShell. I'll leave it up to you, though.)

As for the CRC: I guess they're just hashing the pointer down to a unique value, not actually using it for a data checksum, so that's fine with me. I guess that's useful anyhow, since a CRC will be 32-bits even if the pointer is 64.

[DanielGibson]

pushed some more fixes for most (all?) of the things we discussed here, but there is still more to do, like several more tag-things from BSP.
(probably not today, though).

Thanks @rcgordon, @DrItanium and @yamgent for the feedback! :-)

[DanielGibson]

ok, one more fix for fishy things in Texture.cpp :-)

[icculus]

Merging, thanks!

[rohit-n]

After the merge, the game now segfaults immediately when starting at line 1442 of Texture.cpp. Apparently, QtCreator considers td_pulObjects to have "no such value". (?)

[DanielGibson]

@rohit-n weird.. I have improved (I hope) the stuff in Textures.cpp a bit, can you try out the code from #29 ?

[rohit-n]

As I mentioned in #29, the segfault in Texture.cpp appears to resolved with that change. However, I am now unable to compile on my 32-bit Ubuntu 12.04 machine (x86 ASM disabled) with GCC 4.6.3, due to the changes in Types.h. Specifically, UINTPTR_MAX is not defined. Apparently in my stdint.h, the check for

#if !defined __cplusplus || defined __STDC_LIMIT_MACROS

fails here, so it is never defined.

[DanielGibson]

grrr, why is the world so broken?! :-/

can you try adding #define __STDC_LIMIT_MACROS before #include <stdint.h> in Sources/Engine/Base/Types.h aroudn line 73?

[Yamagi]

From my FreeBSD adventures yesterday I can tell you that defining __STDC_LIMIT_MACROS right before stdint.h in Types.h is not enough. That include statement is not always the first to include stdint.h. Adding it to the top of Engine/StdH.h help in some cases, but not for sources generated by ecc.

I choose the easy route and forced a --std=c++11 into CXXFLAGS. At least with FreeBSDs base system Clang 3.4 UINTPTR_MAX is available / imported when the compiler runs in C++ 11 mode.

[DanielGibson]

not sure if -std=c++11 helps on gcc 4.6 on an old ubuntu from 2012, but it might be worth a try

[rohit-n]

std=c++11 definitely does not work here.

Defining __STDC_LIMIT_MACROS in Types.h gets me linking the 3 .so files, but fails when compiling BaseEvents.es.

In file included from /home/rohit/Code/Serious-Engine/Sources/./Engine/StdH.h:51:0,
                 from /home/rohit/Code/Serious-Engine/Sources/Engine/Classes/BaseEvents.es:18:
/home/rohit/Code/Serious-Engine/Sources/./Engine/Base/Types.h:84:6: error: #error Your system does not provide UINTPRT_MAX, find another way!

I tried defining it again in StdH.h before the Types.h include, but the error persists.

[DanielGibson]

in the CMakeLists.txt try adding add_definitions(-D__STDC_LIMIT_MACROS=1) somewhere, maybe below add_definitions(-D_MT=1)

[rohit-n]

That fixed the issue. Shall I make a PR?

[DanielGibson]

I'm currently writing code to get rid of the dependency on UINTPTR_MAX, so maybe not ;)

[DanielGibson]

now it's like https://github.com/DanielGibson/Serious-Engine/commit/c1b14f9130303d7aac2189f2df8373cdc4108774 I'll not create a pull request yet, wanna do some more real fixes first