Don't use arrays. Replace them with STL containers, usually
std::vector (or
std::array if you really need fixed size without dynamic allocations, which is not the case here).
Pass class objects in general by const-reference.
void pushMessage(const std::string& str, const sf::Color& color=sf::Color::Black);
Use a consistent name convention.
str_in doesn't match the rest.
Combine nested if conditions:
if (event.mouseWheel.delta > 0 && chatLineFocus > 9)
--chatLineFocus;
else if (event.mouseWheel.delta < 0 && chatLineFocus < 19)
++chatLineFocus;
Then, you can use references to create an alias for an often used object:
sf::Text& text = chatText[chatLineFocus - (8 - i)];
text.setPosition(705.f, 550.f + (13.f * float(i)));
mWindow.draw(text);
temp2 = text.getString().toAnsiString();
temp = text.getPosition();
Use more meaningful variable names than "temp". Why do you even need to store the string again? Also consider that
sf::String offers implicit conversions.
Check if a socket receipt is successful, and not only if one specific error occurs.
if (socket.receive(dataPacketFrom) == sf::Socket::Done)
Don't declare variables long before they are used. Declaring all necessary variables at the beginning of a function or block is ancient practice originating from C89 (that's a quarter of a century!). This will already make your code a bit less ugly. Don't declare loop variables such as
i outside the loop. For example:
sf::Glyph glyph = chatFont.getGlyph(lastLine[i],chatText[0].getCharacterSize(),false);
Instead of
for (i=1;i<20;i++)
chatText[i-1] = chatText[i]; //shift all the messages down to accept a new message
you can simply erase the front element of an STL container. Maybe
std::deque, but also
std::vector won't hurt for such few elements.
Instead of
chatText[19].setString(sf::String(str_in)); //put the new message in
you use the container's member function
push_back(). Don't ever refer to magic number indices, use
front() and
back() to highlight the special meaning of these elements. But avoid magic numbers such as
19.
The condition
if (lastSpace != -1)
and its contained statements can be condensed, since only one number is different. Don't hesitate to create new functions that help you for small tasks.
One option is also to separate the graphical update and the
draw() calls. After all, it's unnecessary to recompute all the colors, positions and so on if nothing has changed.
Furthermore, after cleaning up the code, make sure the high-level logic is clearly understandable. This will also help you see if there are mistakes or potential for simplification.
