Vapourware Project X: early tech demo - feedback needed

[potatojohn]

Project X web site:
https://dvolk.github.io/project_x/

The following posts contain a sort of development log of the game. Read it at your own risk.

--

I've decided to try to make a FOSS Neo-scavenger-like.

I started out with the part that I guessed would be most annoying: the grid/mouse interface. Seems it's doable, I've also implemented a scrolling hex grid

Excuse the programmer art.

Anyway, programming on your own is pretty dull, so I'm looking for collaborators. We're men not mice so we'll be using C++ with Allegro 5.0 for cross-platform interfacing.

There's no formal application for joining the merry band, just start contributing on github. If you'd like to contribute but don't want to use github you can send me patches manually (or paste them here). If you'd like to contribute and you don't know github but you would like to know github, I can write up some instructions on how to use it.

Anyone who contributes anything will be immortalized with an entry in the AUTHORS file.

github:

https://github.com/dvolk/project_x

The current code is EXPLORATORY *runs away*

 

[pakoito]

Isn't C++11 too modern for your sophisticated tastes? I was expection you to run C98 without all that useless syntactic sugar from newer versions.

 

[potatojohn]

Isn't C++11 too modern for your sophisticated tastes? I was expection you to run C98 without all that useless syntactic sugar from newer versions.

Nah, I appreciate anything that makes C++ less of a pain. I'm actually not much of a language snob any more. In fact, I initially considered using python and kivy

Progress: I widgetized the UI, and added the button and message log widgets.

 

[Untermensch]

we'll be using C++

No thanks.

 

[DraQ]

Excuse the programmer art.

Admittedly, it's hard to.

What exactly are you trying to make?
Though, TBH I'll probably pour any spare time and programming juices I can find into Pioneer or extend into OpenMW or try to start a project that's been on my brain for some time.

 

[potatojohn]

No thanks.

Everyone uses C++ for games

What exactly are you trying to make?

I'm making a Neo-scavenger-like.

 

[Untermensch]

Everyone uses C++ for games

And if everyone wants to jump of a cliff, would you follow them?

There's zero reasons to use C++ when making a 2D sprite-based game.

 

[pakoito]

Everyone uses C++ for games

I'm making a Neo-scavenger-like.

Neo-scavenger is made in flash

Wasteland 2 in C#.

Legend of Grimrock gameplay in Lua.

 

[28.8bps Modem]

Would it be fair to say you're somewhat new to C++? It pains me how exception unsafe that hunk of code is.
Also, what's with the bizarre C style C++ where you have structs that are half classes, half POD? While C++11 technically allows you to do these things with structs, I don't think it should be seen as permission.

Also:

&grids[i]

No. No. No. Nooooooo. Welcome to segfault city, population you the next time that vector relocates itself.

 

[Hirato]

Tempted to fork and do some cleanup and initial refactoring...

 

[potatojohn]

Also:

&grids[i]

No. No. No. Nooooooo. Welcome to segfault city, population you the next time that vector relocates itself.

Oh yeah.

 

[potatojohn]

I have to admit, I don't fully understand how to mix OOP and memory management. Is there some concise guide I can read if I already know C memory management?

In particular, I had

void switch_ui(bool p) {
  if(!p) {
    TestUI2 ui = new(TestUI2);
    delete (static_cast<TestUI*>(g_ui));
    g_ui = ui
  } else {
    TestUI ui = new(TestUI);
    delete (static_cast<TestUI2*>(g_ui));
    g_ui = ui
  }
}

However this both leaked memory and randomly crashed (in the graphics driver no less)

If we have any C++ experts that could explain what I'm doing wrong that would be great

 

[Hirato]

Why are you type casting g_ui in the first place?

 

[potatojohn]

Why are you type casting g_ui in the first place?

It didn't call the destructors otherwise

 

[Hirato]

It didn't call the destructors otherwise

Oh... make the destructor in the UI class 'virtual'
Also make sure to initialise your pointers to NULL.

As a safe rule of thumb, a class's destructor should ONLY clean up its own members, those of derived and parent types are the responsibility of said derived and parent types.

 

[Hirato]

so to demonstrate, your destructors should be as follows

struct UI {
    ..................
    virtual ~UI(void);
};
..........
UI::~UI(void) {
    info("~UI");
    for(auto* w : widgets)
        delete w;
}
.........

UITest1::~UITest1() { /* no op*/ }
UITest2::~UITest2() { /* no op*/ }

 

[potatojohn]

so to demonstrate, your destructors should be as follows

Oh right. I'm not sure why I put that in the derived classes.

However, I get the same crash (although now it's consistent)

Info: ~TestUI()
Info: ~UI()
Info: ~TileMap
Info: ~Widget
Info: ~MessageLog
Info: ~Widget
Info: ~Button
Info: ~Widget
Info: ~TestGridSystem()
Info: ~GridSystem()
Info: ~Grid()
Info: ~Grid()
Info: ~Widget

Program received signal SIGSEGV, Segmentation fault.
malloc_consolidate (av=av@entry=0x7ffff68a9760 <main_arena>) at malloc.c:4157
4157 malloc.c: No such file or directory.
(gdb) bt
#0 malloc_consolidate (av=av@entry=0x7ffff68a9760 <main_arena>) at malloc.c:4157
#1 0x00007ffff656c0e8 in _int_malloc (av=0x7ffff68a9760 <main_arena>, bytes=32768) at malloc.c:3423
#2 0x00007ffff656e2f0 in __GI___libc_malloc (bytes=32768) at malloc.c:2891
#3 0x00007ffff21fdb25 in ?? () from /usr/lib/dri/fglrx_dri.so
#4 0x00007ffff245eee6 in ?? () from /usr/lib/dri/fglrx_dri.so
#5 0x00007ffff24bba38 in ?? () from /usr/lib/dri/fglrx_dri.so
#6 0x00007ffff24c2888 in ?? () from /usr/lib/dri/fglrx_dri.so
#7 0x00007ffff245c7e7 in ?? () from /usr/lib/dri/fglrx_dri.so
#8 0x00007ffff2480b54 in ?? () from /usr/lib/dri/fglrx_dri.so
#9 0x00007ffff2483d4b in ?? () from /usr/lib/dri/fglrx_dri.so
#10 0x00007ffff24ca915 in ?? () from /usr/lib/dri/fglrx_dri.so
#11 0x00007ffff249dc89 in ?? () from /usr/lib/dri/fglrx_dri.so
#12 0x00007ffff249e0d3 in ?? () from /usr/lib/dri/fglrx_dri.so
#13 0x00007ffff13e17cb in ?? () from /usr/lib/dri/fglrx_dri.so
#14 0x00007ffff14d4814 in ?? () from /usr/lib/dri/fglrx_dri.so
#15 0x00007ffff14d4d71 in ?? () from /usr/lib/dri/fglrx_dri.so
#16 0x00007ffff14be25a in ?? () from /usr/lib/dri/fglrx_dri.so
#17 0x00007ffff14cf45a in ?? () from /usr/lib/dri/fglrx_dri.so
#18 0x00007ffff14cf501 in ?? () from /usr/lib/dri/fglrx_dri.so
#19 0x00007ffff213ecd3 in ?? () from /usr/lib/dri/fglrx_dri.so
#20 0x00007ffff2e558a9 in ?? () from /usr/lib/dri/fglrx_dri.so
#21 0x00007ffff633a48b in glXSwapBuffers () from /usr/lib/fglrx/libGL.so.1
#22 0x0000000000405736 in main () at main.cpp:712

 

[potatojohn]

Hm. Could it be drawing the UI in another thread while I'm deleting it?

 

[Hirato]

I dunno, I mostly use SDL and I roll my own container classes on top of that.
I'm also not noticing anything obviously wrong with the code currently on the repo.

 

[28.8bps Modem]

Hm. Could it be drawing the UI in another thread while I'm deleting it?

Nope, that's what happens in allegro's atexit() routine. What you're doing is deleting a bunch of resources out from under allegro without properly freeing up the display infrastructure you've created.

Hopefully this'll teach you the lesson that comments of the form:

// don't really need to deallocate if we're quitting...

are ALWAYS wrong. Go back and call the destroy routines for every resource you create in reverse order of their creation.

While we're on the subject, nothing you've written takes any account of the exceptions your code might throw. This is especially bad news when you're mixing C with C++. To take an example:

ALLEGRO_BITMAP *button_up = al_load_bitmap("media/buttons/button_up.png"); /* allocation, C style */
ALLEGRO_BITMAP *button_down = al_load_bitmap("media/buttons/button_down.png"); /* allocation, C style */
Button *test_button = new(Button); /* allocation, C++ style */
...
widgets.push_back(test_button); /* if this throws an exception, you've leaked 3 allocations and 2 allegro resources. Oops. Your program is now that much likelier to segfault on exit. */

Any time a naked allocation exists on the stack, and something can throw an exception between the time the allocation is created and it is safely stored somewhere that'll be cleaned up, you need to wrap it in a smart pointer.

For C++ style allocations, you can just say something like:

std::unique_ptr<Button> test_button(new Button()); /* creates our button wrapped in a unique_ptr. */
...
widgets.push_back(test_button.get()); /* stores our pointer somewhere safe, will throw an exception if the storage fails */
test_button.release(); /* now that the pointer is definitely stored safely, and no exception happened we can release our pointer. We know longer want it destroyed with this stack frame. */

For C style code, either you have to directly assign the pointer to an object which will clean it up in its destructor, or you're going to have to write a specialised smart pointer destructor for it.

When you have a declaration like:

vector<ALLEGRO_BITMAP *>bitmaps

You have no choice but to do the latter. That looks like:

class DestroyAllegroBitmap
    {
public:
    void operator()(ALLEGRO_BITMAP* bitmap) const;
    };
  
void DestroyAllegroBitmap::operator()(ALLEGRO_BITMAP* bitmap) const
    {
    al_destroy_bitmap(bitmap);
    }

...
  
std::unique_ptr<ALLEGRO_BITMAP, DestroyAllegroBitmap>(al_load_bitmap("whatever"));

 

[potatojohn]

Why would pushing a pointer into a vector ever throw? If that happens there are some serious issues transcending the game, probably the computer is ablaze and about to explode

I mean I hear you about the smart pointers and all, but wouldn't you agree that they're kind of a pain to use, what with the useless error reporting and all. At least when I crash I can usually get something sensible out of gdb

Anyway, I moved all the loading to the beginning. Now creating an UI just involves pushing widgets into the vector. I also made new buttons

 

[28.8bps Modem]

Why would pushing a pointer into a vector ever throw? If that happens there are some serious issues transcending the game, probably the computer is ablaze and about to explode.

I'm not sure you really understand what a vector is. It's a thin wrapper around an array of the templated type. When that array is full, the array gets reallocated.

To quote from std::vector::push_back's documentation:

If a reallocation happens, the storage is allocated using the container's allocator, which may throw exceptions on failure (for the default allocator, bad_alloc is thrown if the allocation request does not succeed).

The nature of std::vector also suggests you should never be erasing from it in a random access fashion as you do. Erasing from the middle of a vector causes it to be compacted, which is slow. You should only ever add and delete from the end of a vector. More random access erasure friendly data types would include things like std::list and std::set.

 

[potatojohn]

Yes, I know my data structures and algorithms thank you. However in this case there will usually be < 100 items and the removal might happen at most a couple of times per second, so performance is pretty much irrelevant.

 

[potatojohn]

Well, more than a couple of times once there's AI running around doing the same things as the player I guess, but I'll deal with that when it comes up.

 

[potatojohn]

Started adding map characters