ValveSoftware / halflife

Half-Life 1 engine based games
Other
3.63k stars 614 forks source link

SDK VS2012 Compile. #1422

Open Sparky121167 opened 10 years ago

Sparky121167 commented 10 years ago

In pman_triangleffect.h

warning C4018: '<=' : signed/unsigned mismatch Compile error, is this corrected code right?

protected: static bool CheckSize(size_t size) // was (int size) { // This check will help prevent a class frome being defined later, // that is larger than the max size MemoryPool is expecting, // from being successfully allocated. if (size > (unsigned long) CMiniMem::Instance()->MaxBlockSize())

dtugend commented 10 years ago

Are you sure that's why it's not compiling, it's just a "warning" not an "error"? Meaning it should compile despite of the warning. Maybe you overlooked the error(s) in the Output / Error list window?


Btw. there should be lots more of warnings due to the CRT secure deprecation warnings which can be disabled by defining _CRT_SECURE_NO_WARNINGS, however I didn't do that back then. Maybe someone should define it for the projects, since the secure functions are not present on Linux / with gcc and thus should either not be used or would need to be wrapped for Linux.

Sparky121167 commented 10 years ago

Thanks for the reply. Yes I also disabled the _CRT_SECURE_NO_WARNINGS to compile to reduce the warnings in the compile as these warnings do not effect the logic or operation of the code. I am not a programmer but would think that comparing one type against another could throw an exception or rounding logic error, so the warning may in this case be a real error. The solution provided is my guess and may not be right and be more of a problem but I believe current the code is in any case wrong and requires a fix.

dtugend commented 10 years ago

Well it won't throw an exception or s.th.: According to this microsoft doc the compiler will convert the signed type to a unsigned type for comparison. However in extreme situations this may result in unwanted behaviour. And you are right, compiler warnings are there for a reason and should be addressed if possible.

I suggest you suggest your fix as part of a pull request (that is if no-one else sees a problem with your fix, I might have overlooked s.th.*). For doing so, you might find these documents helpful: Fork A Repo Syncing a fork Using Pull Requests

* I can't see a potential problem with your fix, however it might happen that in the actual code your fix is not used, since classes are passed across interfaces to the particleman.dll (ParticleManager), and I am not sure which implementation will be used in such a case (the one you give or the one in particlemanager.dll).