Fix some warnings #45
Conversation
…ons but non-virtual destructor" [-Wdelete-non-virtual-dtor] CButtonAction isn't a base class or derived class so having those functions be virtual didn't achieve anything.
That would never trigger the ASSERT. Now they always do.
…ogical-operand]
There is this other line which uses &:
if (litPacketIter->pa_ubReliable & UDP_PACKET_CONNECT_RESPONSE) {
which is very likely what was intended.
…s false" [-Wsometimes-uninitialized]
…ntheses-equality]
… parentheses" [-Wparentheses]
Thanks @SLAwww (The first parameter to _assert is actually the message, not the expression to evaluate, so "false &&" doesn't belong in there.)
Also changed all "if (this==NULL) return;"s. Fixes some -Wtautological-undefined-compare warnings. Quoting Clang: "'this' pointer cannot be null in well-defined C++ code; comparison may be assumed to always evaluate to false"
Quoting Clang: "reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always evaluate to false"
(I didn't receive any warnings after enabling -Wsign-compare.)
|
oh.. looks like we might have done some duplicate work: #44 |
|
Wow, how is it possible that we opened the PRs only 2 seconds apart 😮 Anyway, I saw you fixed |
| @@ -268,7 +268,7 @@ procedures: | |||
| // declare yourself as a model | |||
| InitAsModel(); | |||
| // fish must not go upstairs, or it will get out of water | |||
| SetPhysicsFlags((EPF_MODEL_WALKING|EPF_HASGILLS)&~EPF_ONBLOCK_CLIMBORSLIDE|EPF_ONBLOCK_SLIDE); | |||
| SetPhysicsFlags(((EPF_MODEL_WALKING|EPF_HASGILLS)&~EPF_ONBLOCK_CLIMBORSLIDE)|EPF_ONBLOCK_SLIDE); | |||
There was a problem hiding this comment.
are you sure this was intended and not (EPF_MODEL_WALKING|EPF_HASGILLS) & ~(EPF_ONBLOCK_CLIMBORSLIDE|EPF_ONBLOCK_SLIDE)?
(I wasn't so I didn't change this in my fix-warnings-branch)
There was a problem hiding this comment.
This seems correct to me.
(EPF_MODEL_WALKING|EPF_HASGILLS) & ~EPF_ONBLOCK_CLIMBORSLIDE
removes "climb or slide" from physical flags, and then we re-add "slide" flag with |EPF_ONBLOCK_SLIDE
There was a problem hiding this comment.
Ok
It's awesome that you look at what the community is doing with your code and clarify issues like this, thanks a lot!
|
There were two competing PRs for this before; @DanielGibson, did we decide this PR should go in, too? Let me know and I'll accept it. |
|
I'd suggest merging my PR first and then cherry-picking the parts of this which aren't duplicates of my fixes - thankfully it seems like @emlai split up the fixes into meaningful commits, like a professional :-) (I can do the cherry picking etc, I could create a new PR with the remaining commits of this one then) |
|
I also wonder why all those |
seeing my stuff is merged now, unless someone beats me to it I plan to do the merging/cherry-picking this weekend |
|
I created a new pull request with all the still relevant fixes from this one, see #54 |
|
Merged #54, closing this one. Thanks, everyone! |
|
wow, that was fast, thanks a lot :) |
No description provided.