Pvs studio fixes #186
Pvs studio fixes #186
Conversation
Fixes issues found by PVS-Studio
Fixed issues found by PVS-Studio
Fixed issues found by PVS-Studio
Fixed issues found by PVS-Studio
Fixed issues found by PVS-Studio
Fixed issues found by PVS-Studio
Fixed issues found by PVS-Studio
In some cases `rootParentAsMOSR` could be accessed first and only then checked for being empty. Fixed issues found by PVS-Studio
`progress` was first checked for being <=0, then it is checked for >0, which cannot occur in the same branch. Fixed issues found by PVS-Studio
`m_pCurrentObject` is first checked for correctness, then re-assigned and then accessed multiple times without checking for nullptr. Fixed issues found by PVS-Studio
There was a problem hiding this comment.
Copilot reviewed 1 out of 16 changed files in this pull request and generated no comments.
Files not reviewed (15)
- Source/Activities/AreaEditor.cpp: Language not supported
- Source/Activities/AssemblyEditor.cpp: Language not supported
- Source/Activities/GibEditor.cpp: Language not supported
- Source/Activities/MultiplayerServerLobby.cpp: Language not supported
- Source/Activities/SceneEditor.cpp: Language not supported
- Source/Entities/ACRocket.cpp: Language not supported
- Source/Entities/ACraft.cpp: Language not supported
- Source/Entities/Attachable.cpp: Language not supported
- Source/Entities/MOSParticle.cpp: Language not supported
- Source/Entities/MOSprite.cpp: Language not supported
- Source/GUI/GUIBanner.cpp: Language not supported
- Source/GUI/GUIFont.cpp: Language not supported
- Source/Menus/MetagameGUI.cpp: Language not supported
- Source/Menus/MetagameGUI.h: Language not supported
- Source/Menus/SceneEditorGUI.cpp: Language not supported
| @@ -269,8 +269,6 @@ bool MOSprite::HitTestAtPixel(int pixelX, int pixelY, bool validOnly) const { | |||
| } | |||
|
|
|||
| void MOSprite::SetFrame(unsigned int newFrame) { | |||
There was a problem hiding this comment.
pretty sure this is lua accessible so I'd rather this accept int instead, to avoid underflow issues
| @@ -126,12 +126,6 @@ void MOSParticle::Travel() { | |||
| m_Atom->ClearMOIDIgnoreList(); | |||
|
|
|||
| if (m_SpriteAnimMode == ONCOLLIDE) { | |||
| // Change angular velocity after collision. | |||
| if (hitCount >= 1) { | |||
There was a problem hiding this comment.
this also makes hitCount unused
There was a problem hiding this comment.
Maybe we should actually return hitcount from Atom travel instead?
Source/Entities/ACraft.cpp
Outdated
There was a problem hiding this comment.
pretty sure this borks delivery in some cases
There was a problem hiding this comment.
yup, those breaks seem to be actually intentional. it's what we like to call Dan ™️ code (overly verbose code from the initial oss release that works mostly by luck)
|
I think some of these really deserve a full refactor, since there's a few in here that are still mostly initial release code. (doesn't have to be here. I'll post up some issues) |
|
Don't want to keep this laying around, but maybe a partial merge makes sense. We'll need to undo the changes to the DanCode™️that're breaking some stuff though. |
Some fixes spotted with PVS studio.
https://pvs-studio.com/en/order/open-source-license/