The engine and several extensions appear to have been written without reading a C++ book, a man page, or indeed the surrounding code

[ViezTrinker]

I spent some time actually reading this codebase. That was a mistake, in the sense that I now know how it works. Below are defects ranging from "this cannot even compile" to "this will crash if you look at it the wrong way." I am including the fixes, because apparently hoping the compiler would invent the correct program was the original strategy.

I am not opening a PR. Consider this a courtesy report for code that has been "actively maintained" since 2018.

---

1. olc::v_3d::operator!= compares a pointer to a float

utilities/olcUTIL_Hardware3D.h

return (this->x != rhs.x || this->y != rhs.y || this != rhs.z);

Yes, that is this, the pointer, compared to rhs.z. operator== correctly compares z. operator!= does not. Instantiating vf3d != vf3d is ill-formed. GCC:

error: invalid operands of types 'const olc::v_3d<float>*' and 'const float' to binary 'operator!='

This is not a subtle template issue. This is a typo that would have been obvious to anyone who used the operator, or compiled with a compiler, or looked at the line.

Fix:

return (this->x != rhs.x || this->y != rhs.y || this->z != rhs.z);

The missing ->z is the entire bug. Congratulations.

---

2. v_3d::lerp calls a member operator* that does not exist

return this->operator*(T(1.0 - t)) + (v1 * T(t));

operator* is a free function. There is no member operator*. The function is constexpr and will not instantiate on a toolchain that actually checks template bodies.

Fix:

return (*this * T(1.0 - t)) + (v1 * T(t));

---

3. GetViewRight / GetViewForward return references to temporaries

const olc::vf3d& GetViewRight() const
{
    return -vecViewRight;
}

-vecViewRight is a temporary. You return a const& to it. The object is dead before the caller can use it. GCC warns about this. The comment on GetViewForward even says // ?? To thunk about. Perhaps finish thinking before shipping it.

Fix: return by value.

olc::vf3d GetViewRight() const
{
    return -vecViewRight;
}

olc::vf3d GetViewForward() const
{
    return -vecViewForward;
}

This is an API change for anyone who took a reference to the return. Anyone doing that was already holding a dangling reference, so they will not miss it.

---

4. Sprite::GetPixel PERIODIC wrapping is wrong for negative coordinates

return pColData[abs(y % height) * width + abs(x % width)];

In C++, (-1) % 4 is -1. abs(-1) is 1. Periodic wrap of -1 on a width of 4 is index 3, not 1.

Reproduced: 4 1 sprite, pixels red/green/blue/white. GetPixel(-1, 0) returned green. It should have returned white. Also, width == 0 or height == 0 divides by zero. A sprite that failed to load is a perfectly normal thing to sample in this engine, apparently.

Fix:

if (width <= 0 || height <= 0)
    return Pixel(0, 0, 0, 0);

const int32_t wrappedX = ((x % width) + width) % width;
const int32_t wrappedY = ((y % height) + height) % height;
return pColData[wrappedY * width + wrappedX];

This is the standard positive modulo. It is taught in first-year programming courses, usually before "write a game engine."

---

5. LoadObj reads index[1] and index[2] without checking tuple size

utilities/olcUTIL_Hardware3D.h

A perfectly legal OBJ face is f 1 2 3. The parser produces tuples of size 1. The loader then does:

if (!texs.empty())
    m.uv.push_back(texs[index[1] - 1].a());

If the file also has vt lines which most of them do this is an out-of-bounds std::vector access. libstdc++ asserts __n < this->size() and aborts.

Empty lines hit sLine[0] on an empty std::string. That is also undefined behavior. OBJ files contain blank lines. I cannot stress this enough: OBJ files contain blank lines.

Fix: skip empty lines; bounds-check each index before use.

if (sLine.empty())
    continue;

// ...
if (!index.empty() && index[0] >= 1 && static_cast<size_t>(index[0]) <= verts.size())
    m.pos.push_back(verts[index[0] - 1].a());
else
    m.pos.push_back({ 0,0,0,0 });

if (index.size() >= 2 && index[1] >= 1 && static_cast<size_t>(index[1]) <= texs.size())
    m.uv.push_back(texs[index[1] - 1].a());
else
    m.uv.push_back({ 0,0 });

if (index.size() >= 3 && index[2] >= 1 && static_cast<size_t>(index[2]) <= norms.size())
    m.norm.push_back(norms[index[2] - 1].a());
else
    m.norm.push_back({ 0,0,0,0 });

---

6. GFX3D OBJ tokenizer starts at nTokenCount = -1

extensions/olcPGEX_Graphics3D.h

int nTokenCount = -1;
while (!s.eof())
{
    char c = s.get();
    if (c == ' ' || c == '/')
    {
        if (tokens[nTokenCount].size() > 0)  // tokens[-1]
            nTokenCount++;
    }
    else
        tokens[nTokenCount].append(1, c);    // also tokens[-1]
}
tokens[nTokenCount].pop_back();              // and then pop_back on EOF garbage

There is a commented-out parser above this that simply does s >> f[0] >> f[1] >> f[2]. Someone replaced working code with a tokenizer that indexes -1, then pop_back()s the EOF character they just stuffed into the string. The array is nine std::strings. Nine.

There is even a comment elsewhere thanking someone for finding an OOB error. The lesson did not take.

Fix: start at 0, stop on EOF, do not pop_back a character you should never have appended, and reject incomplete token sets.

int nTokenCount = 0;
while (nTokenCount < 9)
{
    const char c = s.get();
    if (s.eof())
        break;
    if (c == ' ' || c == '/')
    {
        if (tokens[nTokenCount].size() > 0)
            nTokenCount++;
    }
    else
        tokens[nTokenCount].append(1, c);
}

Also delete[] the old depth buffer in ConfigureDisplay() before allocating a new one. Repeated calls currently leak. new is not a suggestion to forget delete.

---

7. ALSA audio uses the comma operator instead of clip()

extensions/olcPGEX_Sound.h

Windows:

nNewSample = (short)(clip(GetMixerOutput(...), 1.0) * fMaxSample);

ALSA:

nNewSample = (short)(GetMixerOutput(...), 1.0) * fMaxSample;

That is the comma operator. GetMixerOutput is evaluated and discarded. The expression yields 1.0. Every sample is full scale. A mixer value of 0.25 becomes 32767 instead of 8191.

This is not "Linux audio is hard." This is copying a line and deleting the wrong four characters. Users of USE_ALSA have been playing a square wave at maximum amplitude. Charming.

Fix: make the ALSA line identical to the Windows line.

Windows DestroyAudio() also never calls waveOutClose, never frees m_pBlockMemory / m_pWaveHeaders, and never frees loaded samples. InitialiseAudio + DestroyAudio in a loop is a leak. PlaySample(0) indexes vecAudioSamples[-1]. WAV loading reads nHeaderSize bytes into a fixed WAVEFORMATEX with no cap.

I will not write a novella about each of those. The pattern is the same: nothing is checked, nothing is freed.

---

8. Network: operator>> underflows, MessageClient cannot remove clients, release() leaks

extensions/olcPGEX_Network.h

size_t i = msg.body.size() - sizeof(DataType);
std::memcpy(&data, msg.body.data() + i, sizeof(DataType));

If the body is shorter than the type, size_t wraps and you memcpy from another county.

client.reset(); // parameter is by value
m_deqConnections.erase(std::remove(..., client), ...); // looks for nullptr

MessageAllClients resets the deque entry (auto&) and then erases nullptr. MessageClient resets a copy, then searches for nullptr. The dead connection stays in the deque forever. You wrote the correct version in the function immediately below this one.

m_connection.release();

unique_ptr::release() does not delete. It leaks. The comment says "Destroy the connection object." It does not.

header.size from the network is trusted with body.resize(...). A hostile peer can OOM the process. tsqueue::wait() checks empty() without holding the mutex it later waits on. Lost wakeups. This is the textbook condition-variable bug.

Fixes:

if (msg.body.size() < sizeof(DataType))
    return msg;
size_t i = msg.body.size() - sizeof(DataType);

OnClientDisconnect(client);
m_deqConnections.erase(
    std::remove(m_deqConnections.begin(), m_deqConnections.end(), client),
    m_deqConnections.end());

m_connection.reset();

Cap header.size before resize. Wait on the same mutex that guards the deque.

---

9. Raycaster divides by vDirection.x and vDirection.y with no zero check

extensions/olcPGEX_RayCastWorld.h

olc::vf2d vRayDelta = {
    sqrt(1 + (vDirection.y / vDirection.x) * (vDirection.y / vDirection.x)),
    sqrt(1 + (vDirection.x / vDirection.y) * (vDirection.x / vDirection.y))
};

Axis-aligned rays exist. They occur at the edges of the FOV, and whenever the player looks exactly north/south/east/west, which is something players do, constantly, on purpose.

Object overlap resolution divides by fDistance when two centers coincide. Wall sampling divides by fWallHeight when the ray misses. 0 is a number that happens in games.

Fix: if a component is ~0, use a large delta instead of dividing. If fDistance is ~0, skip the push. If fWallHeight is 0, do not divide by it.

---

10. Assorted "I have never heard of bounds checking"

• FillTexturedTriangle indexes [0],[1],[2] with no size() check. FillTexturedPolygon two functions later already has this check. Copy it.
• tstep = 1.0f / (bx - ax) when bx == ax. Guard it. Several copies in PGE and GFX3D.
• olc_UpdateMouseState writes pMouseNewState[button] with no range check. The array is 5 elements.
• olc_UpdateKeyState uses mapKeys[keycode], which inserts unknown keys. ConvertKeycode already uses count/at. Use find.
• ResourcePack::GetFileBuffer uses mapFiles[sFile], which inserts a {0,0} entry for missing files and then reads from offset 0.
• QuickGUI ListBox: std::clamp(..., m_vList.size()-1) on an empty list. size_t underflow. Upper bound becomes SIZE_MAX.
• PopUpMenu ClampCursor: items.size() - 1 on an empty menu. Same joke.

---

Notes for the maintainer

Several of these files still say UNDER ACTIVE DEVELOPMENT - THERE ARE BUGS/GLITCHES. That is not a license to ship tokens[-1], comma-operator audio, or unique_ptr::release() under a comment that says "destroy."

I verified the PERIODIC wrap, operator!=, lerp, LoadObj OOB, and the GFX3D tokenizer against a real compiler (g++ 15, C++20). The ALSA comma expression evaluates to full scale, as the language requires. The rest follows from reading the code, which I recommend as a process.

If the project would like a patch, the diffs are straightforward. I will not be rewriting the tokenizer a third time for you.