View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0005593 | The Dark Mod | Coding | public | 20.04.2021 13:56 | 23.04.2021 05:29 |
Reporter | stgatilov | Assigned To | stgatilov | ||
Priority | low | Severity | trivial | Reproducibility | have not tried |
Status | resolved | Resolution | fixed | ||
Product Version | TDM 2.09 | ||||
Target Version | TDM 2.10 | Fixed in Version | TDM 2.10 | ||
Summary | 0005593: idList::Clear refactoring | ||||
Description | We decide to do the following changes: idList::Clear drops all elements but does not delete them idList::ClearFree deletes all elements along with memory buffer | ||||
Tags | No tags attached. | ||||
I have underestimates the amount of code and how popular Clear is =) It took me two evenings to get through all of idList::Clear usages. I have seen a lot of code which calls Clear when it is not necessary: 1) clearing default-constructed idList member in class constructor 2) clearing idList member in class destructor 3) clearing local idList variable immediately after declaring it (with default constructor called) No idea why. The general policy was to use Clear (non-freeing) everywhere, except for: 1) Use ClearFree in Shutdown / FreeXXX methods 2) Use ClearFree for lists which contain std::shared_ptr inside (I'd rather prefer to NOT have such stuff in game code) 3) Use ClearFree For global variables 4) Sometimes it is also used when list survives for some time, but is obviously never used again... |
|
There are quite a lot of classes which have "Clear" method in game code too. Initially, I tried to rename them to ClearFree if they do free memory. But it only makes the code more complicated, plus there is no uniformity there anyway. So I'll better enforce the strict Clear/ClearFree policy for idlib containers, but leave other Clear methods with their original names. |
|
Committed the changes: r9296. idList now has Clear and ClearFree methods. r9297. Reviewed all calls to idList::Clear, replaced some of them with ClearFree. The second commit is pretty massive. Interestingly, nothing broke even with commit 9296 alone. The changes of 9297 are done in order to "do things right" (at least try to). |
|
Linux compile is broken: /tdm_src/trunk/game/ai/EAS/EAS.cpp:37:20: error: cannot bind non-const lvalue reference of type ‘std::vector<std::shared_ptr<eas::ClusterInfo> >&’ to an rvalue of type ‘eas::tdmEAS::ClusterInfoVector {aka std::vector<std::shared_ptr<eas::ClusterInfo> >}’ _clusterInfo.swap(ClusterInfoVector()); ^~~~~~~~~~~~~~~~~~~ /tdm_src/trunk/game/ai/EAS/EAS.cpp:38:25: error: cannot bind non-const lvalue reference of type ‘std::vector<std::shared_ptr<eas::ElevatorStationInfo> >&’ to an rvalue of type ‘eas::tdmEAS::ElevatorStationVector {aka std::vector<std::shared_ptr<eas::ElevatorStationInfo> >}’ _elevatorStations.swap(ElevatorStationVector()); ^~~~~~~~~~~~~~~~~~~~~~~ |
|
Fixed compile error in svn rev 9298. | |
Thank you! Thus far everything appears to be stable. No crashes or anomalies in any complex missions I've fiddled with. |
|
Next commit (svn rev 9299) is cleaning idHashIndex methods. Now Clear does not delete buffers, while ClearFree do free all memory. svn rev 9300 is about reviewing clearing idDict-s. |
|
Now idStr follows the trend. The first commit is rather unrelated to the main goal --- just fixing something left over from switch to 64-bit mode: r9301. idStr now takes 32 bytes on 64-bit platforms too. Here is the actual renaming: r9302. Renamed idStr::Empty and idStr::Clear. So now clearing a string is "Clear" instead of "Empty". And clearing with dynamic buffer release is called "ClearFree" instead of "Clear". |
|
And two more commits: r9303. Revised Clear methods in more idlib classes. r9304. Reviewed idWinding Clear methods. Now all frequently usable containers from idlib follow the same policy: ClearFree() makes container empty and frees all dynamic memory Clear() makes container empty and retains as much memory buffers as possible Some classes have only one of these methods (whichever makes sense). |
|
Added idList::Reserve in svn rev 9306. | |
Date Modified | Username | Field | Change |
---|---|---|---|
20.04.2021 13:56 | stgatilov | New Issue | |
20.04.2021 13:56 | stgatilov | Status | new => assigned |
20.04.2021 13:56 | stgatilov | Assigned To | => stgatilov |
21.04.2021 15:14 | stgatilov | Note Added: 0013890 | |
21.04.2021 15:26 | stgatilov | Note Added: 0013891 | |
21.04.2021 16:56 | stgatilov | Note Added: 0013892 | |
22.04.2021 02:22 | nbohr1more | Note Added: 0013893 | |
22.04.2021 03:14 | stgatilov | Note Added: 0013894 | |
22.04.2021 04:56 | nbohr1more | Note Added: 0013895 | |
22.04.2021 14:50 | stgatilov | Note Added: 0013896 | |
22.04.2021 15:22 | stgatilov | Note Edited: 0013896 | |
22.04.2021 17:36 | stgatilov | Note Added: 0013897 | |
23.04.2021 03:19 | stgatilov | Note Added: 0013898 | |
23.04.2021 03:19 | stgatilov | Status | assigned => resolved |
23.04.2021 03:19 | stgatilov | Resolution | open => fixed |
23.04.2021 03:19 | stgatilov | Fixed in Version | => TDM 2.10 |
23.04.2021 05:29 | stgatilov | Note Added: 0013899 |