Refactors to the code #1467
Refactors to the code #1467
Conversation
|
Yeah so I've already fucked up my clean commit list... 😁😁 |
| { | ||
| const bool bDrawModeChanging = (m_CurDrawMode != newDrawMode); | ||
| const bool bBlendModeChanging = (m_CurBlendMode != newBlendMode); | ||
|
|
||
| // Flush old | ||
| if (m_CurDrawMode == EDrawMode::DX_SPRITE) | ||
| { | ||
| m_pDevice->SetSamplerState(0, D3DSAMP_ADDRESSU, D3DTADDRESS_WRAP); | ||
| m_pDevice->SetSamplerState(0, D3DSAMP_ADDRESSV, D3DTADDRESS_WRAP); | ||
| m_pDXSprite->End(); | ||
| } | ||
| else if (m_CurDrawMode == EDrawMode::DX_LINE) | ||
| { | ||
| m_pLineInterface->End(); | ||
| } | ||
| else if (m_CurDrawMode == EDrawMode::TILE_BATCHER) | ||
| { | ||
| m_pTileBatcher->Flush(); | ||
| } | ||
| else if (m_CurDrawMode == EDrawMode::PRIMITIVE) | ||
| { | ||
| m_pPrimitiveBatcher->Flush(); | ||
| } | ||
| else if (m_CurDrawMode == EDrawMode::PRIMITIVE_MATERIAL) | ||
| { | ||
| m_pPrimitiveMaterialBatcher->Flush(); | ||
| } | ||
| // Draw mode changing? | ||
| if (!bDrawModeChanging && !bBlendModeChanging) | ||
| return; | ||
| } |
There was a problem hiding this comment.
What is the extra scope here for? Also there's not really a need for const here
There was a problem hiding this comment.
well, just wanted to indicate neither of the 2 bools are used later. Putting the in a scope indicates that.
…ning the wrong value
… emplace_back, not push_back
…k instead of inserting copying the QueueItem
…aphics::DrawCircleQueued
|
This PR is |
|
I agree. Ill open separate PRs. |
|
|
||
| CVector2D operator+(const CVector2D& vecRight) const { return CVector2D(fX + vecRight.fX, fY + vecRight.fY); } | ||
| constexpr CVector2D operator+(const CVector2D& vecRight) const noexcept { return CVector2D(fX + vecRight.fX, fY + vecRight.fY); } | ||
|
|
||
| CVector2D operator-(const CVector2D& vecRight) const { return CVector2D(fX - vecRight.fX, fY - vecRight.fY); } | ||
| constexpr CVector2D operator-(const CVector2D& vecRight) const noexcept { return CVector2D(fX - vecRight.fX, fY - vecRight.fY); } | ||
|
|
||
| CVector2D operator*(const CVector2D& vecRight) const { return CVector2D(fX * vecRight.fX, fY * vecRight.fY); } | ||
| constexpr CVector2D operator*(const CVector2D& vecRight) const noexcept { return CVector2D(fX * vecRight.fX, fY * vecRight.fY); } | ||
|
|
||
| CVector2D operator/(const CVector2D& vecRight) const { return CVector2D(fX / vecRight.fX, fY / vecRight.fY); } | ||
| constexpr CVector2D operator/(const CVector2D& vecRight) const noexcept { return CVector2D(fX / vecRight.fX, fY / vecRight.fY); } | ||
|
|
||
| void operator+=(float fRight) | ||
| constexpr void operator+=(float fRight) noexcept | ||
| { | ||
| fX += fRight; | ||
| fY += fRight; |
There was a problem hiding this comment.
I don't think noexcept is needed here. The CVector classes don't throw any exceptions. Also, I feel like constexpr is not useful here since the result is usually not known at compile time. The compiler is smart enough to know when to inline a function. This is just making the code hard to read.
There was a problem hiding this comment.
please comment to #1478, and lets discuss it there.
tbh I agree, but nobody reads CVector code anyways. Constexpr is needed(at least for the constructor) in CGraphics refactor(somewhere in the end, the sphere thing)
|
As noted above, this is getting disorganized. I'll close it, and do separate PRs for the ease of testing, and merging. |
5 participants
Alright, here I'll try to refactor a lot of code, while in the same time having meaningful commits.
Im creating this PR, so the changes can be reviewed if needed.
Also, Im trying not to refactor the code too much, so I dont introduce bugs, because that woudnt be fun.
Mostly I refactor loop to use 'range based for loop' I also add a lot of 'auto', and 'constexpr' where needed.