[aquaria] crash :/

Mathias Panzenböck grosser.meister.morti at gmx.net
Tue Apr 26 13:54:52 EDT 2011


On 04/26/2011 07:03 PM, Andrew Church wrote:
>> So all IngrediantData* fields should be replaced with std::string fields?
>
> With respect to the food holders, yes.  (I don't want to comment on other
> uses of IngredientData at the moment since I haven't checked them in
> detail.)  Then you'd have a lookup call in each function that currently
> dereferences the stored pointer.
>
>> When I cook something else and then combine two leaves twice I couldn't reproduce the crash myself.
>> I guess I have found a certain inventory state that shows this behaviour by accident. I attached the
>> save state that causes the problem. Just immediately open the cooking menu and combine two leaves twice.
>
> Thanks!  I confirmed the crash here -- it looks like the cause in this
> case is that the first action on the inventory is cooking a new item,
> which causes the push_back() call to expand (and thus reallocate) the
> inventory array.
>

Exactly. However, there are also other methods that change the array in which the IngredientData 
instances are stored (e.g. sort them) and thus invalidate all the pointers to IngredientData 
objects. Using a smart pointer I simply eliminated the problem without having to understand much of 
the code. :) I just ensured that copies of the IngredientData instances are made where it was 
explicitly done before. This happens in 2 places in Aquaria/Continuity.cpp:
In Continuity::loadFile where Continuity::pickupIngredient is called and again in 
Continuity::pickupIngredient itself. I emulated this behaviour using code equivalent to:
SmartPtr<IngredientData> data = new IngredientData(*d);

I guess that the only place where a copy should be made is when a IngredientData gets from 
Continuity::ingredientData to Continuity::ingredients?

Also I guess that the signature of a lot (but not all) of methods that currently have a 
`IngredientData*` (or `const IngredientDataPtr&` in my version) parameter should be changed to 
`const std::string&` as well?

Here I compiled a list of where I used IngredientDataPtr[1]. Note that all of these classes hold 
pointers to IngredientData instances. For the usage in combination with std::vector it before where 
direct instances. Please mark where it should be a std::string and where it should be a direct 
instance. Basically say which class(es) should be the owner(s) of IngredientData instances. I 
suspect Continuity, because it is the only class where IngredientData instances are created. They 
are updated in other classes as well, though.

[1] typedef SmartPtr<IngredientData> IngredientDataPtr;

There are other places where IngredientData* is used and I didn't change the usage, because the 
pointers are only used during one method invocation and are not changed or stored anywhere (e.g. 
Continuity::getIEString). In these cases I thing it would be better to use `const IngredientData&`, 
because it makes it clear that the ingredient data will not be changed and no reference will be 
stored anywhere.

With according instructions I'll make the changes.

RecipeMenuEntry:
   IngredientDataPtr data

Ingredient:
   IngredientDataPtr data
   Ingredient(const Vector &pos, const IngredientDataPtr &data, int amount=1)
   IngredientDataPtr getIngredientData()

FoodSlot:
   IngredientDataPtr ingredient
   IngredientDataPtr lastIngredient
   IngredientDataPtr getIngredient()

FoodHolder
   IngredientDataPtr foodHolderIngredient
   void setIngredient(const IngredientDataPtr &i, bool effects=true)
   IngredientDataPtr getIngredient()

Game:
   std::vector<IngredientDataPtr> cookList
   void spawnIngredientFromEntity(Entity *ent, const IngredientDataPtr &data)
   Recipe *findRecipe(const std::vector<IngredientDataPtr> &list)
   void pickupIngredientEffects(const IngredientDataPtr &data)

Entity:
   IngredientDataPtr ingredientData

Continuity:
   std::vector<IngredientDataPtr> ingredientData
   std::vector<IngredientDataPtr> ingredients
   bool isIngredientFull(const IngredientDataPtr &data)
   bool pickupIngredient(const IngredientDataPtr &i, bool effects=true)
   IngredientDataPtr getIngredientHeldByName(const std::string &name)
   IngredientDataPtr getIngredientDataByName(const std::string &name)
   IngredientDataPtr getIngredientByIndex(int idx)
   IngredientDataPtr getIngredientDataByIndex(int idx)


	-panzi


More information about the aquaria mailing list