SFML community forums

General => SFML projects => Topic started by: Otherian on August 26, 2015, 07:07:36 pm

Title: MapMaker - Startproject for SFML
Post by: Otherian on August 26, 2015, 07:07:36 pm
Hello Everyone,

so I've started playing around with SFML (and C++) the last days and wanted to get some feedback on my Code. I've created some sort of Tile-Map-Creator (similar to the one in the tutroial) which can build a 2D-Map out of Triangles and squares based on a level-matrix (and textur-matrix).

I thought two classes would be appropriate:
a) TileMap wich contains the texture-image and position data of certain texture-sections
b) Map Entity, wich loads the World (also drawn in EventLoop)

What do you guys think?
Am I doing something completely wrong or does it look ok to you? :)

Pictures:
Example-2D World (http://i.imgur.com/Z5tMff0.png?1)
Used Tileset (http://i.imgur.com/pKo4AM7.png?1) (Credits: Cave Story; With little Explanation of Variable-Names)

Code:
>> GitHub (https://github.com/Otherian/SFML-MapMaker/tree/master)
Title: Re: MapMaker - Startproject for SFML
Post by: silverweed on September 02, 2015, 11:58:35 am
Is the code on Github supposed to work? There's a typo (unclosed string) in this file (https://github.com/Otherian/SFML-MapMaker/blob/master/Map.hpp), and you're using
#include <SFML\Graphics.hpp>
instead of
#include <SFML/Graphics.hpp>
. Also, Map has its draw() method defined private, which makes little sense, and the naming convention you're using is incoherent: I suggest you choose either camelCase, PascalCase or c_like_naming and stick with that (when working with SFML the camelCase is the most natural choice).
Title: Re: MapMaker - Startproject for SFML
Post by: fallahn on September 02, 2015, 02:02:48 pm
A private draw function is perfectly sensible when inheriting from sf::Drawable. (http://www.sfml-dev.org/tutorials/2.3/graphics-vertex-array.php#creating-an-sfml-like-entity)
Title: Re: MapMaker - Startproject for SFML
Post by: SpeCter on September 02, 2015, 02:42:05 pm
Is the code on Github supposed to work? There's a typo (unclosed string) in this file (https://github.com/Otherian/SFML-MapMaker/blob/master/Map.hpp), and you're using
#include <SFML\Graphics.hpp>
instead of
#include <SFML/Graphics.hpp>
. Also, Map has its draw() method defined private, which makes little sense, and the naming convention you're using is incoherent: I suggest you choose either camelCase, PascalCase or c_like_naming and stick with that (when working with SFML the camelCase is the most natural choice).

Like fallahn said already, the private draw function is perfectly fine and the includes you mention are fine too.
The include you don't mention however is exactly the reason for the unclosed string error, because there is one additional quote #include "Tileset.hpp"" <-----------
Title: Re: MapMaker - Startproject for SFML
Post by: silverweed on September 02, 2015, 04:23:00 pm
Like fallahn said already, the private draw function is perfectly fine and the includes you mention are fine too.
The backslashes aren't really "fine" actually: from the C++11 specs you read (paragraph 2.9/2):

Quote from: C++11Specs
The appearance of either of the characters ’ or \ or of either of the character sequences /* or // in a q-char-sequence or an h-char-sequence is conditionally supported with implementation-defined semantics, as is the appearance of the character " in an h-char-sequence.

which in short means that using '\' instead of '/' leads to not much more than undefined behaviour ("implementation-defined" is pretty explicit on this).

As for the private draw(), you're right: I didn't know it was friend with RenderTarget, so mea culpa.

Quote from: SpeCter
The include you don't mention however is exactly the reason for the unclosed string error, because there is one additional quote #include "Tileset.hpp"" <-----------
That's what I mentioned here:
Quote from: silverweed
There's a typo (unclosed string) in this file
Title: Re: MapMaker - Startproject for SFML
Post by: Nexus on September 02, 2015, 05:16:21 pm
which in short means that using '\' instead of '/' leads to not much more than undefined behaviour ("implementation-defined" is pretty explicit on this).
Don't confuse implementation-defined and undefined behavior, they mean different things. See here (http://stackoverflow.com/q/2397984) for an explanation of the terminology.

That is, using backslashes in include paths is permitted on certain implementations, but not advised for portable coding. There's no reason not to use forward slashes which work everywhere.
Title: Re: MapMaker - Startproject for SFML
Post by: silverweed on September 02, 2015, 07:20:49 pm
which in short means that using '\' instead of '/' leads to not much more than undefined behaviour ("implementation-defined" is pretty explicit on this).
Don't confuse implementation-defined and undefined behavior, they mean different things. See here (http://stackoverflow.com/q/2397984) for an explanation of the terminology.

That is, using backslashes in include paths is permitted on certain implementations, but not advised for portable coding. There's no reason not to use forward slashes which work everywhere.
I'm aware of the difference in terminology, that's why I said:
Quote
to not much more than undefined behaviour
I meant to stress the fact that it's not a portable feature: what works on some platform/with some compiler may not work on others, and since it's so trivial to avoid this unreliability it's something worth fixing.
Title: Re: MapMaker - Startproject for SFML
Post by: SpeCter on September 03, 2015, 08:50:29 am
Argh sorry silverweed, I somehow overlooked the "and" in your sentence and thought you meant that the '/' '\' thing is supposed to lead to the unclosed string error, my bad.
Title: Re: MapMaker - Startproject for SFML
Post by: silverweed on September 03, 2015, 11:59:50 am
Argh sorry silverweed, I somehow overlooked the "and" in your sentence and thought you meant that the '/' '\' thing is supposed to lead to the unclosed string error, my bad.
No problem, I could've expressed myself more clearly ;)
Title: Re: MapMaker - Startproject for SFML
Post by: Otherian on September 08, 2015, 07:39:35 pm
Uh, I didnt even notice someone answered here... sorry, I was kinda busy the last days.
Thx for the feedback all :)

@silverweed
The Code on GitHub was ofc supposed to work and it did on my PC.
I wasnt aware of a difference between "/" and "\" so I didnt give much attention to it, but will fix this in the future. As stated already: Private-Draw fct works cause inheritance :)
I have a lot of problems with naming because I just started programming. I can't choose a right "way" for me... gues it's highly recommended to use one and stick with it and I should get this done asap T_T


Unfortunately I deleted the old Project from GitHub the last days because I started from scratch again to get it a little bit more structured. (besides the old one was either way just a "how to sfml" for me xD)

New things are:
> tried to include the concept of State-Machienes
> maker-surface to select tileshape / texture / ...
> it's now possible to actually map yourself with the desired shapes etc ^^''
> Save / Load levels
> 5 different TileLayers
> variable MapSize
> zoom, move, etc your map
> MapGrid
> ...

Currently Disabled:
> Tile-Size-Option
> In-game-Visibility

To-Do:
> Cleaning up some unnecessary functions in my classes and choosing a appropriate name-convention for my variables
(right now it's a huge mess because I just started to code and didnt look back  ;D)
> Alpha-Support for texture
> Whole-Mode Menu
>> Draw: The one currently working
>> Select: Selecting multiple tiles with the mouse to fill all / delete all / Ctrl-X / ...
>> Objects: Special Objects like animated stuff / enemys / ...
> variable Texture-Size (currently the texture-pic has to be 720x720 with 72x72 per tile)

------------------------------
GitHub:
Click me ^^ (https://github.com/Otherian/SFML-MapMaker)

Video:
>> Youtube (https://youtu.be/_FZl1KC_WZo) (I'm too dumb to create a useable GIF without tons of artifacts...)

Pictures:
(http://i.imgur.com/9iYygGr.png)
Title: Re: MapMaker - Startproject for SFML
Post by: Hapax on September 08, 2015, 11:58:50 pm
In your picture, "F" would've been better to use in in "SFML"  :P
Title: Re: MapMaker - Startproject for SFML
Post by: FRex on September 09, 2015, 02:41:33 am
I tried to compile it with clang++ on Fedora but you still use \ in include paths and then I had some other errors so I just gave up... :-\
Title: Re: MapMaker - Startproject for SFML
Post by: Otherian on September 09, 2015, 09:18:04 am
In your picture, "F" would've been better to use in in "SFML"  :P
Kay, fixed it :P

I tried to compile it with clang++ on Fedora but you still use \ in include paths and then I had some other errors so I just gave up... :-\
Mäh, I didnt update the repository yesterday after reading about the "/" and "\" stuff... my bad. Finally fixed it now ^^''
What other errors do you get? I can't look into them if I dont know whats going wrong at other PCs. Could you make a screen of your compiler-log or smth?
Title: Re: MapMaker - Startproject for SFML
Post by: FRex on September 09, 2015, 04:14:24 pm
I also get this kind of error in TileMap.hpp :
http://stackoverflow.com/questions/11692806/error-extra-qualification-student-on-member-student-fpermissive
Other than that it builds and works.
Title: Re: MapMaker - Startproject for SFML
Post by: Otherian on September 09, 2015, 05:54:51 pm
I also get this kind of error in TileMap.hpp :
http://stackoverflow.com/questions/11692806/error-extra-qualification-student-on-member-student-fpermissive
Other than that it builds and works.
Wupsi, looks like a little bit too much Ctrl+C, Ctrl+V ^^''
I wonder why my compiler isn't saying anything about such kinds of errors. Guess it's "correcting" them intern but not even a warning occurred :<
Nevertheless, thx for your answers. I hope it works now :)


I had a general problem in my design and maybe someone can help. In principle my project should work like this:

(http://i.imgur.com/CItG888.png)

The problem is: For example only the "TileMap-Class" (or maybe just the "Map-Class") should have informations about the actual level openend, the tile-number in x and y direction, tilesize, etc... but often I found it neccessary to access these informations also in the "MakerObjectManager-Class". One can simply say: Well, just make a GetXX()-Function and all should be fine, but I'm not sure about if its better to create such a function and call it everytime some information is needed or -and thats how I implemented it in the end- to create an own member for the variable in this classes, call the GetXX()-function ONCE and than only access this member again and again.
Is there some general rule how to approach this issue? Is it better to create an own member or call the GetFct each time you need the variable?  :o

Sorry for my bad english, I hope one can understand my question ^^
Title: Re: MapMaker - Startproject for SFML
Post by: Elias Daler on September 09, 2015, 07:07:18 pm
You should totally use getters and setters. Copying the member once is a bad idea, because it leads to a copy which wouldn't update if the original changes. If you really need to have a reference to this object, you should better use std::shared_ptr instead.
If you are spending lots of time calling getters and setters, you should think about it, maybe this member should be in a class which uses this member variable. Maybe your class is doing to much stuff and the class which holds member functions which you access frequently needs to have a member function which would do the stuff with those variables!

For example, you may have a code like this:

if(someObject.getX() > y) {
    someObject.setX(y);
}

Then maybe you can create a function which looks like this:

void SomeClass::changeXValue(Something& y) {
    if(x > y) {
         x = y;
    }
}

If you can't find a way to do this and you can make some members public or make a Struct instead of Class.
For example, class Level changes tiles a lot, that's why I have struct Tile, so I can do stuff like this in the Level member functions:

tile.x = 10;
tile.y = 20;

I strongly recommend against using friend classes, they can lead to a code that is hard to change.
For example, before I introduced chunks in levels, I stored all tiles in one std::vector<std::vector>>
I have LevelEditor class which needs to change tiles a lot. I got tired of using stuff like this:

level->setTile(x, y, id);
 
So I've decided to make class LevelEditor a friend class for Level, so I could do stuff like this instead:

level->tiles[y][x] = id;
 
This was a bad idea, because when I've decided to store tiles in chunks, all tiles were not stored in one 2d array anymore! They were stored in N (N - number of chunks) 2d arrays!
So, every line of code which used tiles vector directly, didn't work anymore.
Some other classes used setTile function. So I only changed this function to work with chunks and not 2d array and every piece of code worked without any modifications. This was a pretty valuable lesson for me. :)

Title: Re: MapMaker - Startproject for SFML
Post by: Otherian on September 10, 2015, 12:14:33 am
You should totally use getters and setters. Copying the member once is a bad idea, because it leads to a copy which wouldn't update if the original changes.
Yeah, I had quite some trouble with this problem. You forget to change both member-variables once and the whole thing reacts quite strange to certain inputs. So I already imagined that it's bad to copy some specific variables but I thought calling a getter too many times isn't a desired solution either. ^^''

If you really need to have a reference to this object, you should better use std::shared_ptr instead.
If you are spending lots of time calling getters and setters, you should think about it, maybe this member should be in a class which uses this member variable. Maybe your class is doing to much stuff and the class which holds member functions which you access frequently needs to have a member function which would do the stuff with those variables!
I haven't done much with shared_ptr's but I'm planing to experiment with them the next days. In generel they are like normal pointers but delete the object they are pointing at if deleted / overrwritten with another object-adress?! Nevertheless it looks like I have to rethink my whole structure and organise which class is capable of doing certain tasks and split the different functions accordingly. Preferably this would have been done from the start but oh well... *hehe*

One question arises though:
For example if I want to stick with my Tile- and TileMap-Class (the latter has a map with all tiles and maintains them etc). Is there a smart way that my Tiles can get informations like their current size etc from the superior TileMap-Class? The other way round I would just create a getter in Tile-Class but this doesnt work in this direction. As you mentioned a smart way would be to just use Tiles as a struct in TileMap to have immediate access to all variables neccessary but I'm just curious if thers a clever way to solve this in general :P

Thx for your feedback Elias :)