Use move for std::vector instead of copying in path functions #1895
Conversation
xezon
left a comment
xezon
left a comment
There was a problem hiding this comment.
Did you check that the passed vector is never used after it is consumed?
| #else | ||
| // TheSuperHackers @performance bobtista 23/11/2025 Use swap to emulate move semantics for VC6 compatibility | ||
| parms.m_coords.swap(*path); | ||
| path->clear(); |
There was a problem hiding this comment.
This clear() is redundant. path will already be empty.
There was a problem hiding this comment.
pathwill already be empty.
Why would it be?
There was a problem hiding this comment.
Because AICommandParms parms(...) is created with empty m_coords.
There was a problem hiding this comment.
I think making the code clearer at a 'local' level can't hurt, but this is now probably a moot point with a swap / move helper function.
There was a problem hiding this comment.
You are not wrong, it is reasonable to make the intent clear, but in that case a comment would be better then instead of adding a redundant function call.
| AICommandParms parms(AICMD_FOLLOW_PATH, cmdSource); | ||
| parms.m_coords = *path; | ||
| #if __cplusplus >= 201103L | ||
| parms.m_coords = std::move(*path); |
There was a problem hiding this comment.
Keep it simple and use swap even for c++11
There was a problem hiding this comment.
Swap is unnecessary here and does not convey the intended operation: a single move operation; we don't care about the contents (if anything) of parms.m_coords.
There was a problem hiding this comment.
Yes but then we do not need all this #if __cplusplus >= 201103L stuff here.
Otherwise, create a "move" helper somewhere else that we use here, which does move in c++11 and swap in C++98.
There was a problem hiding this comment.
Ok I added a helper to CppMacros.h
There was a problem hiding this comment.
Ok that works. I think CppMacros.h needs renaming to to cpp_compat.h later :-)
xezon
added
Minor
Looking at call sites, they all pass local vectors (exitPath, path) that are not used after the function call. They go out of scope immediately, so this is safe, right? |
|
Yes, when the std::vector contents are not used anymore after they are now consumed by the functions then this is safe. |
|
|
||
| // TheSuperHackers @performance bobtista 25/11/2025 Helper to move-assign from pointer: uses std::move in C++11, swap in C++98 | ||
| template<typename T> | ||
| inline void move_assign_from_pointer(T& dest, T* src) |
|
|
||
| // TheSuperHackers @performance bobtista 25/11/2025 Helper to move-assign from pointer: uses std::move in C++11, swap in C++98 | ||
| template<typename T> | ||
| inline void move_assign_from_pointer(T& dest, T* src) |
| #include "GameLogic/Damage.h" | ||
| #include "Common/STLTypedefs.h" | ||
| #include "ref_ptr.h" | ||
| #include "Utility/CppMacros.h" |
There was a problem hiding this comment.
This include should not be necessary. It should already be included by other means.
3 participants
Refactors path-following functions to use move when transferring
std::vector<Coord3D>ownershipChanges:
aiFollowExitProductionPathandaiFollowPathinline functions to accept non-const vector pointers and usestd::moveAIStateMachine::setGoalPathto move the vector instead of copyingprivateFollowPathanddoQuickExitsignatures to support move semantics