cpp+sfml 'inventory' code optimization

[palo]

hello, i've been working on my fun project, nothing big, just wanted to make interactive inventory window. i'm a newbie, so if anybody would like to share any advices, or tricks how to optimize my code, i would be thankfull!

#include <SFML/Window.hpp>
#include <SFML/Graphics.hpp>
#include <iostream>
using namespace std;
using namespace sf;

struct tile
{int x0,y0,f;};

int main()
{RenderWindow win(VideoMode(500,500,32),"Inventory");
tile ar[6][6];
tile t;

for(int i=0;i<6;i++){//insert coordinates to tiles
for(int j=0;j<6;j++){
tile t={j*100-100,i*100-100,0};
ar[i][j]=t;}}

for(int i=0;i<6;i++){//insert something
t=ar[i][i];
t.f=1;
ar[i][i]=t;}

while(win.isOpen()){
win.clear();
Event event;
while(win.pollEvent(event)){

if(event.type==Event::Closed){win.close();}//close

{int mx0,my0;//interaction with LMB
if(event.type==Event::MouseButtonPressed&&event.mouseButton.button==Mouse::Left){
mx0=Mouse::getPosition(win).x;
my0=Mouse::getPosition(win).y;}
if(event.type==Event::MouseButtonReleased&&event.mouseButton.button==Mouse::Left){

int mx1=Mouse::getPosition(win).x;
int my1=Mouse::getPosition(win).y;
bool filled=false;
for(int i=0;i<6;i++){//check if destination is filled
for(int j=0;j<6;j++){
t=ar[i][j];
if(mx1>t.x0&&mx1<(t.x0+100)){
if(my1>t.y0&&my1<(t.y0+100)){
if(t.f==1)
filled=true;
}}}}
if(filled==false){

int mx01=mx0-mx0%100;
mx1-=mx1%100;
int my01=my0-my0%100;
my1-=my1%100;
int mx=mx1-mx01;
int my=my1-my01;//snap to grid
int vx,vy;
for(int i=0;i<6;i++){
for(int j=0;j<6;j++){
t=ar[i][j];
if(mx0>t.x0&&mx0<(t.x0+100)){//if mouse was pressed on filled tile
if(my0>t.y0&&my0<(t.y0+100)){
if(t.f!=0){
t.x0+=mx;
vx=t.x0;
t.y0+=my;
vy=t.y0;
}}}
ar[i][j]=t;}}
for(int i=0;i<6;i++){//prevent overlapping tiles
for(int j=0;j<6;j++){
t=ar[i][j];
if(vx==t.x0&&vy==t.y0)
t.f=1;
ar[i][j]=t;}}
}}}}


for(int i=0;i<6;i++){//drawing tiles
for(int j=0;j<6;j++){
t=ar[i][j];
RectangleShape rec(Vector2f(100,100));
rec.setPosition(t.x0,t.y0);
if(t.f==1){rec.setFillColor(Color::Red);}
if(t.f==0){rec.setFillColor(Color::Black);}
win.draw(rec);}}

win.display();}
return 0;}

 

[eXpl0it3r]

Hope this is not overwhelming, but there's a lot of style issues and some best practices that should be followed:

• Don't use using namespace but actually type the namespaces for all the types.
• Don't try to use "one liner" codes, it's not readable, instead put every { or } on a new line and every statement on a new line.
• Indent your code!
• Use a space before and a space after an operator, e.g. bla < 10.
• Declare each variable individual instead of clustering them, e.g. int x1;<newline>int x2;<newline>
• Use std::vector instead of C-arrays or at least std::array.
• Don't use magic numbers in loop iterations, but use the size of the containers you're iterating.
• Consider using range-base for loops to iterate containers.
• Use the x and y position of the mouse provided by the event and don't request it via sf::Mouse::getPosition().
• Use an sf::Vector when you want to remember both x and y of something.
• Don't compare booleans against true or false, but instead just do if(bla) or if(!bla).
• ...

In general, I advice you to look at some code style guide to see how people tend to write and understand code and/or to enable auto-formatting in your IDE, so you end up with code that's actually readable for everyone else.

Keep on working and good luck!

[palo]

Thanks a lot man! i tried optimizating code, and this is what i came with:

#include <SFML/Window.hpp>
#include <SFML/Graphics.hpp>
#include <iostream>
using namespace std;
using namespace sf;

struct tile
{int x0,y0,f;};

int main()
{RenderWindow win(VideoMode(500,500,32),"Inventory");
vector< vector<tile> > v(10, vector<tile>(10));
tile t;
int w=50;
for(int i=0;i<v.size();i++){//insert coordinates to tiles
for(int j=0;j<v.size();j++){
tile t={j*w,i*w,0};
v[j][i]=t;}}

for(int i=0;i<v.size();i++){//insert something
t=v[i][i];
t.f=1;
v[i][i]=t;}

while(win.isOpen()){
win.clear();
Event event;
while(win.pollEvent(event)){

if(event.type==Event::Closed){win.close();}//close

int mx0,my0;//interaction with LMB
if(event.type==Event::MouseButtonPressed&&event.mouseButton.button==Mouse::Left){
mx0=Mouse::getPosition(win).x;
my0=Mouse::getPosition(win).y;}
if(event.type==Event::MouseButtonReleased&&event.mouseButton.button==Mouse::Left){
int mx1=Mouse::getPosition(win).x;
int my1=Mouse::getPosition(win).y;

Vector2i m0(mx0,my0);
Vector2i m1(mx1,my1);
m0.x-=m0.x%w;//snap to grid
m0.y-=m0.y%w;
m1.x-=m1.x%w;
m1.y-=m1.y%w;

Vector2i m0t(m0.x/w,m0.y/w);//tile number
Vector2i m1t(m1.x/w,m1.y/w);


t=v[m1t.x][m1t.y];//check if destination is filled
if(t.f!=1){

t=v[m0t.x][m0t.y];
if(t.f!=0){//if mouse was pressed on filled tile, move it

t.f=0;
v[m0t.x][m0t.y]=t;
t=v[m1t.x][m1t.y];//prevent overlapping tiles
t.f=1;
v[m1t.x][m1t.y]=t;}
}}}


for(int i=0;i<v.size();i++){//drawing tiles
for(int j=0;j<v.size();j++){
t=v[i][j];
RectangleShape rec(Vector2f(w,w));
rec.setPosition(t.x0,t.y0);
if(t.f==1){rec.setFillColor(Color::Red);}
if(t.f==0){rec.setFillColor(Color::Black);}
win.draw(rec);}}

win.display();}
return 0;}
 

Don't use using namespace but actually type the namespaces for all the types.
Don't try to use "one liner" codes, it's not readable, instead put every { or } on a new line and every statement on a new line.
Indent your code!
Use a space before and a space after an operator, e.g. bla < 10.
Declare each variable individual instead of clustering them, e.g. int x1;<newline>int x2;<newline>

i understand that for many its less readable but i prefer it that way, because its faster to type and i dont have any problem reading it. Or is there a meaning for it all, which i dont know, but i'm willing to learn!

Use std::vector instead of C-arrays or at least std::array.
Don't use magic numbers in loop iterations, but use the size of the containers you're iterating.
Consider using range-base for loops to iterate containers.
Use an sf::Vector when you want to remember both x and y of something.
Don't compare booleans against true or false, but instead just do if(bla) or if(!bla).

changed it all like you said, idk if well

Use the x and y position of the mouse provided by the event and don't request it via sf::Mouse::getPosition().

i tried my best, but couldn't get this function working :/