0

I want to make simple AI using FSM. When I simply make object it works perfectly fine but when I put them in vector it crashes after second update. Here is smallest code that shows this problem

class State
{
public:
    virtual ~State() = default;

    void setContext(Context* context);

    virtual void processState() = 0;

protected:
    Context* m_Context;

};
void State::setContext(Context* context)
{
    m_Context = context;
}
class State1 : public State
{
public:
    virtual void processState() override;

};
void State1::processState()
{
    std::cout << "Processing State1" << std::endl;
    m_Context->setState(new State2);
}
class State2 : public State
{
public:
    virtual void processState() override;

};
void State2::processState()
{
    std::cout << "Processing State2" << std::endl;
    m_Context->setState(new State1);
}
class Context
{
public:
    Context(State* state);

    void update();

    void setState(State* state);

private:
    State* m_State;

};
Context::Context(State* state)
{
    setState(state);
}

void Context::update()
{
    m_State->processState();
}

void Context::setState(State* state)
{
    delete m_State;

    m_State = state;
    m_State->setContext(this);
}

If I make it like this

Context context(new State1);
context.update();
context.update();
context.update();
context.update();
context.update();

it works and prints

Processing State1
Processing State2
Processing State1
Processing State2
Processing State1

but if I make it like this

std::vector<Context> contexts;
size_t amount = 1;
contexts.reserve(amount);
for (size_t i = 0; i < amount; i++)
{
    Context context(new State1);
    contexts.push_back(context);
}

for (size_t i = 0; i < amount; i++)
{
    for (size_t j = 0; j < 5; j++)
    {
        contexts[i].update();
    }
}

it only prints first state, crashes on second and gives this error code

Processing State1

-1073741819.

I tried to use break points but i still don't understand what's going on

XYZW
  • 1
  • 3
    The pointer that get's `delete`d in `setState` -- can you point out the line in the shown code that `new`ed this pointer in the first place? – Sam Varshavchik Sep 18 '20 at 19:25
  • 2
    ^^ partly a trick question. You should not need to search your code for `new`s and `delete`s and puzzle whether they match. Use smart pointers instead – 463035818_is_not_an_ai Sep 18 '20 at 19:30
  • You mean i can't just create temporary object like this and then push_back? I don't really know how it should be made exactly. I thought it's fine this way if it prints first state. – XYZW Sep 18 '20 at 19:31
  • 1
    Possible duplicate https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three – Galik Sep 18 '20 at 19:34
  • 1
    Your `Context` class violates the rule of 3/5/0 – drescherjm Sep 18 '20 at 19:38
  • 1
    `m_State->setContext(this);` stores a pointer to Context, then you push the Context onto the vector which creates another Context and the State has a dangling pointer. That won't be good. – Eljay Sep 18 '20 at 19:51
  • So I should try making custom copy constructor or/and use smart pointers? – XYZW Sep 18 '20 at 19:56

1 Answers1

0

You should consider using smart pointers or at least initiate the pointers to nullptr and check for this before calling delete.

The value of m_State is garbage, and in some cases you're calling delete on this unknown value - this results in an undefined behaviour and this is the reason that for some cases it works but for other it crashes.

joepol
  • 744
  • 6
  • 22
  • I had it coded but it worked exactly the same so i abandoned it for simplicity. I will try smart pointers then – XYZW Sep 18 '20 at 19:41
  • @Kamadol you consider raw pointers simpler than smart pointers? They aren't. Correctly managing memory with raw pointers is among the most difficult things, just don't do it, it has only downsides and no up – 463035818_is_not_an_ai Sep 18 '20 at 19:50
  • Yeah I know it but I wanted to try it this way. Raw pointers are most of the time pain to use – XYZW Sep 18 '20 at 19:54