MagicFlare refactors#292
Conversation
bsxf-47
force-pushed
the
interactive
branch
5 times, most recently
from
73578ff to
97060bb
Compare
| z += FLARELINESTEP; | ||
| if(!io) { | ||
| z = long(z * g_sizeRatio.y); | ||
| z *= long(g_sizeRatio.y); |
There was a problem hiding this comment.
This looks wrong, long(g_sizeRatio.y) is going to change abruptly from 1 (height 480 - 959) to 2 (height 960 to 1439) to 3 (1440p+) etc. instead of scaling the y offset smoothly.
| z += FLARELINESTEP; | ||
| if(!io) { | ||
| z = long(z * g_sizeRatio.y); | ||
| z *= long(g_sizeRatio.y); |
There was a problem hiding this comment.
Same, I don't think the change of rounding from after to before the multiplication is wanted here.
| void MagicFlareContainer::removeAll() { | ||
|
|
||
| for(size_t i = 0; i < g_magicFlaresMax; i++) { | ||
| MagicFlare& flare = g_magicFlares[i]; |
There was a problem hiding this comment.
This should be MagicFlare & flare
| RenderMaterial mat; | ||
| mat.setBlendType(RenderMaterial::Additive); | ||
|
|
||
| EERIE_LIGHT* light = lightHandleGet(torchLightHandle); |
There was a problem hiding this comment.
This should be EERIE_LIGHT * light
| } | ||
|
|
||
| long MagicFlareCountNonFlagged() { | ||
| long MagicFlareCountWithoutIO() { |
There was a problem hiding this comment.
IO → Entity would be even better
| long count = 0; | ||
| for(size_t i = 0; i < m_magicFlaresMax; i++) { | ||
| if(g_magicFlares[i].exist && !g_magicFlares[i].hasIO) { | ||
| if(g_magicFlares[i].exist && !g_magicFlares[i].io) { |
There was a problem hiding this comment.
Intentional? .io is cleared when the entity is destructed, .hasIO is not. Not sure if this actually matters.
There was a problem hiding this comment.
As far as I know it was like this in the original code, but I removed member variable altogether in a later commit.
|
|
||
| if(EERIE_LIGHT * light = lightHandleGet(handle)) { | ||
| light->m_exists = false; | ||
| if(light) { |
There was a problem hiding this comment.
This is redundant, the first if already checks the same.
There was a problem hiding this comment.
Probably a brain fart. Removed.
| if(flare.exist) { | ||
| removeFlare(flare); | ||
| } | ||
| removeFlare(flare); |
There was a problem hiding this comment.
Worried that his might still call io->flarecount-- for already-cleared flares. Perhaps the io member should also be reset in MagicFlare::clear()
There was a problem hiding this comment.
The flare is made to completely reset in a later commit.
| , size(0.f) | ||
| , io(nullptr) | ||
| , bDrawBitmap(false) | ||
| { } |
There was a problem hiding this comment.
These are redundant if already initialized to the same values in-line.
|
Sorry this took me a few months to get back to. All of the pointed out issues should be sorted now. |
dscharrer
force-pushed
the
master
branch
5 times, most recently
from
391b7f7 to
3d8a7e6
Compare
2 participants
Attempted to put the whole module into a more readable shape. Kept the C style array for speed and the header functions so that there isn't yet another global object floating about.