supertuxkart-sourceforge-migration / stk-migration-test2

0 stars 0 forks source link

Auto-saving Grand-Prix state + Resuming Grand-Prix's. #948

Open supertuxkart-sourceforge-migration opened 10 years ago

supertuxkart-sourceforge-migration commented 10 years ago

Author: unitraxx

We want to give the user the option to resume a Grand-Prix. Suggested solution :

-Auto-save between tracks. (This should be notified to the user, as he wants to know that he can resume his GP later on.)

-State will be stored in the user's config file.

-We have two options to present it to the user :

1) When starting a new GP, a option should be given to resume a saved one or start a new one. 2) List the option 'Resume GP' between the track selection list.

Open for discussion.

Migrated-From: https://sourceforge.net/apps/trac/supertuxkart/ticket/948

supertuxkart-sourceforge-migration commented 10 years ago

Author: hikerstk I am taking the ticket to avoid that another mentor check it.

supertuxkart-sourceforge-migration commented 10 years ago

Author: hikerstk I get a crash on windows, because in the 2nd race of a GP some freed memory is used. Trigger is the line:

void SavedGP::setKarts(std::vector<RaceManager::KartStatus> kart_list){
    m_karts.clearAndDeleteAll();

clearAndDeleteAll calls the destructor of UserConfigParams, but (because of the new nesting code) it does NOT remove those freed instanced from the list of children of a group. You will (likely) not see this problem on windows, since even though the memory is freed, it is still around unmodified. In VS in debug mode freed memory is overwritten with some nonsense, so you notice this.

Stack trace for the actual crash:

    supertuxkart_wiiuse_d.exe!GroupUserConfigParam::write(XMLWriter & stream={...})  Line 106 + 0x2d bytes  C++
    supertuxkart_wiiuse_d.exe!UserConfig::saveConfig()  Line 728 + 0x3f bytes   C++
    supertuxkart_wiiuse_d.exe!RaceManager::next()  Line 514 C++

Or the line

        m_children[1);

with values:

-       m_children  [10](n]->writeInner(stream,)(0x02ed6ec0 {m_param_name=<Bad Ptr> m_comment="" },0x02ed72a8 {m_param_name=<Bad Ptr> m_comment=<Bad Ptr> },0x02ed7750 {m_param_name=<Bad Ptr> m_comment=<Bad Ptr> },0x02ed7ba0 {m_param_name=<Bad Ptr> m_comment=<Bad Ptr> },0x02ed7ff0 {m_param_name=<Bad Ptr> m_comment=<Bad Ptr> },0x02ed6ea8 {m_attributes=[{m_value="beagle" m_default_value="beagle" },0x02ed6f98 {m_value=19 m_defaul ,...) std::vector<UserConfigParam *,std::allocator<UserConfigParam *> >
+       [0](7](0x02ed6f14)  0x02ed6ec0 {m_param_name=<Bad Ptr> m_comment="" }   UserConfigParam *
+       [{m_param_name=<Bad Ptr> m_comment=<Bad Ptr> }  UserConfigParam *
+       [2](1]  0x02ed72a8) 0x02ed7750 {m_param_name=<Bad Ptr> m_comment=<Bad Ptr> }    UserConfigParam *
+       [{m_param_name=<Bad Ptr> m_comment=<Bad Ptr> }  UserConfigParam *
+       [4](3]  0x02ed7ba0) 0x02ed7ff0 {m_param_name=<Bad Ptr> m_comment=<Bad Ptr> }    UserConfigParam *
+       [{m_attributes=[7](5]   0x02ed6ea8)(0x02ed6f14 {m_value="beagle" m_default_value="beagle" },0x02ed6f98 {m_value=19 m_default_value=19 },0x02ed6fe4 {m_value=0 m_default_value=0 },0x02ed7030 {m_value=-842150451 m_default_value=-842150451 },0x02ed707c {m_value=-1 m_default_value=-1 },0x02ed70c8 {m_value=1.2360001 m_default_value=1.2360001 },0x02ed7114 {m_value="" m_default_value="" }) m_children=[}  UserConfigParam *
+       [6](0]())   0x1cfb04a8 {m_attributes=[{m_value="wilber" m_default_value="wilber" },0x1cfb0598 {m_value=19 m_default_value=19 },0x1cfb05e4 {m_value=1 m_default_value=1 },0x1cfb0630 {m_value=-842150451 m_default_value=-842150451 },0x1cfb067c {m_value=-1 m_default_value=-1 },0x1cfb06c8 {m_value=1.3859999 m_default_value=1.3859999 },0x1cfb0714 {m_value="" m_default_value="" }) m_children=[0](7](0x1cfb0514)() }   UserConfigParam *
+       [{m_attributes=[7](7]   0x1cd9f010)(0x1cd9f07c {m_value="konqi" m_default_value="konqi" },0x1cd9f100 {m_value=10 m_default_value=10 },0x1cd9f14c {m_value=2 m_default_value=2 },0x1cd9f198 {m_value=-842150451 m_default_value=-842150451 },0x1cd9f1e4 {m_value=-1 m_default_value=-1 },0x1cd9f230 {m_value=1.6360000 m_default_value=1.6360000 },0x1cd9f27c {m_value="" m_default_value="" }) m_children=[}    UserConfigParam *
+       [8](0]())   0x1d056d78 {m_attributes=[{m_value="tux" m_default_value="tux" },0x1d056e68 {m_value=4 m_default_value=4 },0x1d056eb4 {m_value=3 m_default_value=3 },0x1d056f00 {m_value=-842150451 m_default_value=-842150451 },0x1d056f4c {m_value=-1 m_default_value=-1 },0x1d056f98 {m_value=2.0860000 m_default_value=2.0860000 },0x1d056fe4 {m_value="" m_default_value="" }) m_children=[0](7](0x1d056de4)() }   UserConfigParam *
+       [{m_attributes=[7](9]   0x1cfb1358)(0x1cfb13c4 {m_value="nolok" m_default_value="nolok" },0x1cfb1448 {m_value=4 m_default_value=4 },0x1cfb1494 {m_value=0 m_default_value=0 },0x1cfb14e0 {m_value=-842150451 m_default_value=-842150451 },0x1cfb152c {m_value=0 m_default_value=0 },0x1cfb1578 {m_value=9.8190002 m_default_value=9.8190002 },0x1cfb15c4 {m_value="" m_default_value="" }) m_children=[0]() }   UserConfigParam *

That's for a GP with 5 karts - the first 5 entries are invalid, and 5 new ones are added.

Some more comments: The opening { goes on a new line according to our coding style ;)

Also I don't like:

SavedGPKart::SavedGPKart(GroupUserConfigParam * group,
   ...) : 
    m_group("Kart", group, "Saved state of a kart"),
    m_ident(ident.c_str(),"ident", &m_group),
    m_score(score,"score",&m_group),

Reason is the usage of m_group when initialising the other members. the C++ standard requires that the initialisation is done in the order in which the variables are declared(!), not the order in which you write them in the initialisation line. This means that if at anytime someone cosmetically changes the grand_prix.h file and changes the order in which the members are declared, the code will actually break (though most compiler give a warning).

So better option would be to do the initialisation in the constructor code (if possible).

supertuxkart-sourceforge-migration commented 10 years ago

Author: hikerstk Thanks, applied in r12782.

A few comments (prepare to get heap of conflicts when you update):

Thanks a lot for the patch! Joerg

supertuxkart-sourceforge-migration commented 10 years ago

Author: hikerstk Still todo: