-
Notifications
You must be signed in to change notification settings - Fork 67
[GEN][ZH] Add compilation for Linux #522
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
0526867
to
2b15bb6
Compare
e33d6eb
to
f668cf3
Compare
6eae27b
to
c588356
Compare
Is there a reason, why |
float det; | ||
#ifdef _WIN32 | ||
D3DXMatrixInverse((D3DXMATRIX *)&mat4Inv, &det, (D3DXMATRIX*)&mat4); | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think some alternative implementation should be provided or some kind runtime error should be produced...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll use GLM here (as we do on our fork). However first i want to integrate VCPKG (possibly a separate PR). For linux to work we need a few external libraries and VCPKG makes that a lot easier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this specific line is that, but what happens in the code at many places is a row major to column major matrix conversion or vice versa. We polished this in Thyme and it needs to be done in this code as well. This needs to be done in a separate change because it is bigger change. Here is the reference:
TheAssemblyArmada/Thyme@2114738
Comment from that change:
// When converting Matrix4 to D3DMATRIX or vice versa always use conversion function below.
// Reason being, D3DMATRIX is row major matrix, and Matrix4 is column major matrix.
// Thus copying from one to another will always require a transpose (or invert).
I have opened task with #552
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference this is the GLM implementation we have: https://github.com/Fighter19/CnC_Generals_Zero_Hour/blob/develop/GeneralsMD/Code/Libraries/Source/WWVegas/WWMath/matrix3d.cpp#L567
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@feliwir I have no doubts that you plan to implement it later. :) My point was more about approach to unimplemented code. I think there should be some unified way how to handle this, right from the start, otherwise there is a danger of silently broken code, if it gets forgotten. Also It would be nice to be able to search for unimplemented code. I have created issue, where I formulated this in more detail.
void Lock_Mem_Log_Mutex(void) | ||
{ | ||
#ifdef _WIN32 | ||
void * mutex = Get_Mem_Log_Mutex(); | ||
#ifdef DEBUG_CRASHING | ||
int res = | ||
#endif | ||
WaitForSingleObject(mutex,INFINITE); | ||
WWASSERT(res==WAIT_OBJECT_0); | ||
_MemLogLockCounter++; | ||
#endif | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think some alternative implementation should be provided or some kind runtime error should be produced...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use std::mutex here, however i'm not sure if the memlogging is used much? When searching for function invocations i only get a few, which don't need any locking mechanism (only ww3d.cpp
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just replace this version of wwmemlog with the version from ZH which seemingly is more linux friendly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't look at that one yet, but when we do dedup first, this probably will resolve itself
Yes, otherwise it comflicts with GCC standardlibrary, which includes a header named |
82ca1d3
to
60d9e2e
Compare
60d9e2e
to
a53a8f4
Compare
Co-authored-by: Patrick Zacharias <1475802+Fighter19@users.noreply.github.com>
Co-authored-by: Patrick Zacharias <1475802+Fighter19@users.noreply.github.com>
Co-authored-by: Patrick Zacharias <1475802+Fighter19@users.noreply.github.com>
Co-authored-by: Patrick Zacharias <1475802+Fighter19@users.noreply.github.com>
Co-authored-by: Patrick Zacharias <1475802+Fighter19@users.noreply.github.com>
Co-authored-by: Patrick Zacharias <1475802+Fighter19@users.noreply.github.com>
Co-authored-by: Patrick Zacharias <1475802+Fighter19@users.noreply.github.com>
Co-authored-by: Patrick Zacharias <1475802+Fighter19@users.noreply.github.com>
Co-authored-by: Patrick Zacharias <1475802+Fighter19@users.noreply.github.com>
Co-authored-by: Patrick Zacharias <1475802+Fighter19@users.noreply.github.com>
a53a8f4
to
820c397
Compare
Note, that this is related to my PR to make code compile with MinGW toolchain as GCC compiler is used in both cases. I think it would make most sense for this to go on top of MinGW support, because:
btw. build fixes for GCC also slightly overlap with VC warning fixes (+ need asm replacements). |
Will be redone after commonizing |
Based on #479
Targets fixed:
Generals:
Zero Hour: