Skip to content

Commit cf21766

Browse files
KelwanKelwan
committed
Code review, removing older styles of coding
1 parent 54e0c9a commit cf21766

File tree

4 files changed

+67
-85
lines changed

4 files changed

+67
-85
lines changed

‎ecsact/runtime/serialize.hh

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,6 @@ inline std::vector<std::byte> serialize(const ecsact_action& action) {
7070
return out_action;
7171
}
7272

73-
// NOTE: Add the wrapper for serialize/derserialize here
74-
7573
/**
7674
* Calls `ecsact_deserialize_action` or `ecsact_deserialize_component` based on
7775
* the type of @tp T.
@@ -142,6 +140,7 @@ int deserialize(
142140
return read_amount;
143141
}
144142

143+
// NOTE: Document functions
145144
inline ecsact_action deserialize(
146145
const ecsact_action_id& id,
147146
std::vector<std::byte>& serialized_action

‎reference/async_reference/async_reference.hh

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
#include <atomic>
99
#include <optional>
1010
#include <variant>
11+
#include "ecsact/runtime/core.hh"
12+
#include "ecsact/runtime/async.h"
1113

1214
#include "reference/async_reference/util/types.hh"
1315
#include "reference/async_reference/util/util.hh"
@@ -16,9 +18,6 @@
1618
#include "reference/async_reference/callbacks/async_callbacks.hh"
1719
#include "reference/async_reference/entity_manager/entity_manager.hh"
1820

19-
#include "ecsact/runtime/core.hh"
20-
#include "ecsact/runtime/async.h"
21-
2221
class async_reference {
2322
public:
2423
ecsact_async_request_id enqueue_execution_options(
@@ -48,6 +47,7 @@ private:
4847
entity_manager entity_manager;
4948

5049
std::thread execution_thread;
50+
std::mutex execution_m;
5151

5252
std::atomic_bool is_connected = false;
5353
std::atomic_bool is_connected_notified = false;

‎reference/async_reference/tick_manager/tick_manager.cc

Lines changed: 38 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -25,71 +25,58 @@ tick_manager::move_and_increment_tick() {
2525
}
2626

2727
types::async_error tick_manager::validate_pending_options() {
28-
if(pending_tick_map.size() > 0) {
29-
std::unique_lock lk(tick_m);
28+
auto result = types::async_error{.error = ECSACT_ASYNC_OK, .request_ids = {}};
29+
30+
if(pending_tick_map.empty()) {
31+
return result;
32+
}
33+
34+
std::unique_lock lk(tick_m);
3035

31-
auto pending_options = std::move(pending_tick_map);
32-
pending_tick_map.clear();
33-
lk.unlock();
36+
auto pending_options = std::move(pending_tick_map);
37+
pending_tick_map.clear();
38+
lk.unlock();
3439

35-
for(auto& [key, pending_list] : pending_options) {
36-
auto merged_options = types::cpp_execution_options{};
40+
for(auto& [key, pending_list] : pending_options) {
41+
auto merged_options = types::cpp_execution_options{};
3742

38-
for(int i = 0; i < pending_list.size(); i++) {
39-
merged_options = {};
43+
for(int i = 0; i < pending_list.size(); i++) {
44+
merged_options = {};
4045

41-
auto pending = pending_list[i];
46+
auto pending = pending_list[i];
4247

43-
auto error = util::validate_options(pending.options);
48+
result.error = util::validate_options(pending.options);
4449

45-
if(error != ECSACT_ASYNC_OK) {
46-
return types::async_error{
47-
.error = error,
48-
.request_ids = {pending.request_id},
49-
};
50-
}
50+
if(result.error != ECSACT_ASYNC_OK) {
51+
result.request_ids.push_back(pending.request_id);
52+
return result;
53+
}
5154

52-
error = util::validate_merge_options(merged_options, pending.options);
55+
result.error =
56+
util::validate_merge_options(merged_options, pending.options);
5357

54-
if(error != ECSACT_ASYNC_OK) {
55-
return types::async_error{
56-
.error = error,
57-
.request_ids = {pending.request_id},
58-
};
59-
}
58+
if(result.error != ECSACT_ASYNC_OK) {
59+
result.request_ids.push_back(pending.request_id);
60+
return result;
61+
}
6062

61-
util::merge_options(merged_options, pending.options);
63+
util::merge_options(merged_options, pending.options);
6264

63-
lk.lock();
64-
error = upsert_validated_tick(merged_options, key);
65-
lk.unlock();
65+
result.error = upsert_validated_tick(merged_options, key);
6666

67-
if(error != ECSACT_ASYNC_OK) {
68-
auto requests = util::get_request_ids_from_pending_exec_options(
69-
pending_options.at(tick)
70-
);
67+
if(result.error != ECSACT_ASYNC_OK) {
68+
auto requests = util::get_request_ids_from_pending_exec_options(
69+
pending_options.at(tick)
70+
);
7171

72-
std::vector<ecsact_async_request_id> request_ids;
73-
request_ids
74-
.insert(request_ids.end(), requests.begin(), requests.end());
72+
result.request_ids
73+
.insert(result.request_ids.end(), requests.begin(), requests.end());
7574

76-
return types::async_error{
77-
.error = error,
78-
.request_ids = request_ids,
79-
};
80-
}
75+
return result;
8176
}
8277
}
83-
84-
return types::async_error{
85-
.error = ECSACT_ASYNC_OK,
86-
.request_ids = {},
87-
};
8878
}
89-
return types::async_error{
90-
.error = ECSACT_ASYNC_OK,
91-
.request_ids = {},
92-
};
79+
return result;
9380
}
9481

9582
ecsact_async_error tick_manager::upsert_validated_tick(
@@ -98,14 +85,9 @@ ecsact_async_error tick_manager::upsert_validated_tick(
9885
) {
9986
if(validated_tick_map.contains(requested_tick)) {
10087
auto existing_options = validated_tick_map[requested_tick];
101-
auto error =
102-
util::validate_merge_options(existing_options, validated_options);
103-
if(error != ECSACT_ASYNC_OK) {
104-
return error;
105-
}
88+
return util::validate_merge_options(existing_options, validated_options);
10689
} else {
10790
validated_tick_map.insert(std::pair(requested_tick, validated_options));
91+
return ECSACT_ASYNC_OK;
10892
}
109-
110-
return ECSACT_ASYNC_OK;
11193
}

‎reference/async_reference/util/util.cc

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,8 @@ ecsact_async_error validate_merge_instructions(
4747
return ECSACT_ASYNC_OK;
4848
}
4949

50-
auto components_range = std::ranges::views::all(components);
51-
auto entities = util::get_cpp_entities(components_range);
52-
53-
auto other_components_range = std::ranges::views::all(other_components);
54-
auto other_entities = util::get_cpp_entities(other_components_range);
50+
auto entities = util::get_cpp_entities(components);
51+
auto other_entities = util::get_cpp_entities(other_components);
5552

5653
auto empty_view = std::ranges::empty_view<int>{};
5754

@@ -63,23 +60,23 @@ ecsact_async_error validate_merge_instructions(
6360
}
6461

6562
for(auto& component : components) {
66-
auto view = std::views::filter(
67-
other_components_range,
68-
[&other_components_range,
63+
auto component_view = std::views::filter(
64+
other_components,
65+
[&other_components,
6966
&component](types::cpp_execution_component other_component) {
7067
return component._id == other_component._id;
7168
}
7269
);
70+
7371
int same_component_count = 0;
7472
int same_entity_count = 0;
75-
for(auto itr = view.begin(); itr != view.end(); itr++) {
73+
for(auto& other_component : component_view) {
7674
same_component_count++;
77-
if(component.entity_id == itr->entity_id) {
75+
if(component.entity_id == other_component.entity_id) {
7876
same_entity_count++;
7977
}
8078
if(same_component_count > 0 && same_entity_count > 0) {
8179
return ECSACT_ASYNC_ERR_EXECUTION_MERGE_FAILURE;
82-
break;
8380
}
8481
}
8582
}
@@ -95,7 +92,7 @@ ecsact_execution_options util::cpp_to_c_execution_options(
9592

9693
if(options.actions.size() > 0) {
9794
std::vector<ecsact_action> deserialized_actions;
98-
deserialized_actions.resize(options.actions.size());
95+
deserialized_actions.reserve(options.actions.size());
9996
for(auto& action_info : options.actions) {
10097
auto action =
10198
ecsact::deserialize(action_info.action_id, action_info.serialized_data);
@@ -107,12 +104,13 @@ ecsact_execution_options util::cpp_to_c_execution_options(
107104
c_options.actions_length = deserialized_actions.size();
108105
}
109106
if(options.adds.size() > 0) {
110-
auto adds_range = std::ranges::views::all(options.adds);
111-
auto entities_view = util::get_cpp_entities(adds_range);
107+
auto entities_view = util::get_cpp_entities(options.adds);
112108

113109
std::vector entities(entities_view.begin(), entities_view.end());
114110

115111
std::vector<ecsact_component> component_list;
112+
// NOTE: Don't resize and push back
113+
component_list.reserve(options.adds.size());
116114

117115
for(int i = 0; i < options.adds.size(); i++) {
118116
auto& component_info = options.adds[i];
@@ -128,12 +126,12 @@ ecsact_execution_options util::cpp_to_c_execution_options(
128126
c_options.add_components_length = options.adds.size();
129127
}
130128
if(options.updates.size() > 0) {
131-
auto updates_range = std::ranges::views::all(options.updates);
132-
auto entities_view = util::get_cpp_entities(updates_range);
129+
auto entities_view = util::get_cpp_entities(options.updates);
133130

134131
std::vector entities(entities_view.begin(), entities_view.end());
135132

136133
std::vector<ecsact_component> component_list;
134+
component_list.reserve(options.updates.size());
137135

138136
for(int i = 0; i < options.updates.size(); i++) {
139137
auto component_info = options.updates[i];
@@ -149,17 +147,16 @@ ecsact_execution_options util::cpp_to_c_execution_options(
149147
c_options.add_components_length = options.updates.size();
150148
}
151149
if(options.removes.size() > 0) {
152-
auto removes_range = std::ranges::views::all(options.removes);
153-
154-
auto entities_view = util::get_cpp_entities(removes_range);
150+
auto entities_view = util::get_cpp_entities(options.removes);
155151
std::vector entities(entities_view.begin(), entities_view.end());
156152

157-
auto component_ids = util::get_cpp_component_ids(removes_range);
158-
std::vector<ecsact_component_id> component_list;
159-
component_list
160-
.insert(component_list.end(), component_ids.begin(), component_ids.end());
153+
auto component_ids_view = util::get_cpp_component_ids(options.removes);
154+
std::vector component_ids(
155+
component_ids_view.begin(),
156+
component_ids_view.end()
157+
);
161158

162-
c_options.remove_components = component_list.data();
159+
c_options.remove_components = component_ids.data();
163160
c_options.remove_components_entities = entities.data();
164161
c_options.remove_components_length = options.removes.size();
165162
}
@@ -170,6 +167,10 @@ types::cpp_execution_options util::c_to_cpp_execution_options(
170167
const ecsact_execution_options options
171168
) {
172169
auto cpp_options = types::cpp_execution_options{};
170+
cpp_options.adds.reserve(options.add_components_length);
171+
cpp_options.updates.reserve(options.update_components_length);
172+
cpp_options.removes.reserve(options.remove_components_length);
173+
cpp_options.actions.reserve(options.actions_length);
173174

174175
if(options.add_components != nullptr) {
175176
for(int i = 0; i < options.add_components_length; i++) {

0 commit comments

Comments
 (0)