Remove the fireball limit #3224
Conversation
|
Yessss |
code/fireball/fireballs.cpp
Outdated
| constexpr int INTITIAL_FIREBALL_SIZE = 256; | ||
|
|
||
| SCP_vector<fireball> Fireball_vec(INTITIAL_FIREBALL_SIZE); | ||
| SCP_vector<int> Unused_fireball_indices(INTITIAL_FIREBALL_SIZE); |
There was a problem hiding this comment.
Careful with the vector constructor call. It adds that many elements and does not only reserve as much. Since you already have the relevant code to reserve the required size below, you can just use the default constructor here.
code/fireball/fireballs.cpp
Outdated
|
|
||
| num = obj->instance; | ||
| fb = &Fireballs[num]; | ||
| // some temporary asserts until we're sure the new system works fine. |
There was a problem hiding this comment.
Why temporary? More consistency checks are always good 😉
code/fireball/fireballs.cpp
Outdated
| } | ||
|
|
||
|
|
||
| if (n == static_cast<int>(Fireball_vec.size())) { |
There was a problem hiding this comment.
If you keep the fireball a pointer and just point that to the correct element around line 787 then you can avoid having that very specific check here.
code/fireball/fireballs.h
Outdated
|
|
||
| extern fireball Fireballs[MAX_FIREBALLS]; | ||
| extern int Num_fireballs; | ||
| extern SCP_vector<fireball> Fireball_vec; |
There was a problem hiding this comment.
The name should stay as Fireballs for consistency with the rest of the code. That _vec suffix isn't really necessary.
code/scripting/api/libs/mission.cpp
Outdated
| int count = 0; | ||
| for (auto& current_fireball : Fireball_vec) { | ||
| if (current_fireball.objnum >= 0) { | ||
| count++; | ||
| } | ||
| } |
There was a problem hiding this comment.
Since this relies on the internal implementation of the fireballs, it should be moved to within the fireball module and only called here via a function.
There was a problem hiding this comment.
OK this all sounds easy to do. Thanks for the review!
There was a problem hiding this comment.
Take another look if you can! Changes should be included.
JohnAFernandez
force-pushed
the
Vectorize-fireballs-maybe
branch
from
58c3348 to
98fa18b
Compare
code/fireball/fireballs.cpp
Outdated
| { | ||
| int count = static_cast<int>(Fireballs.size() - Unused_fireball_indices.size()); | ||
|
|
||
| if (count < 0) { |
There was a problem hiding this comment.
This is a surprisingly simple way of doing this. Maybe add a comment in this function to explain why this works to make understanding this a bit easier.
Also, be careful with your casting range checks. The way this currently works means that this will not be hit in the cases you want. Since both size calls return a size_t, you will get an underflow if Unused_fireball_indices is larger than Fireballs. If you do the casts individually then the result will be as you expect it.
Also, is this a case you would expect? I can't think of something where this could happen without a programming error in which case an assertion would be sufficient.
There was a problem hiding this comment.
I am not expecting it, I just got paranoid. So you're right, I should make it an assert.
JohnAFernandez
added
cleanup
(Without the main PC, so really hoping I don't do any typos)
3 participants
Vectorizes the fireball limit. Thanks to @asarium for some great suggestions. In draft until we get a proper stress test from @EatThePath
For #2945