code help please

[will_asher]

I'll admit I didn't read all the code, but I think I see your problem. Your code looks broadly like this:

list_of_objects = []
for each tile in dungeon {
  append objects in tile to list_of_objects
  if monster is in tile and monster is a mimic {
    fake_object = makeObjectFromMonster(monster)
    append fake_object to list_of_objects
  }
}

The problem is that fake_object goes out of scope when the if statement closes, rendering it invalid. Future created objects then can use the memory that was used by fake_object, writing random stuff to it which will cause a crash when you later try to read it.

If the fake object goes out of scope, then why don't the real objects go out of scope? It looks to me like both the fake objects and the real objects are assigned to
object_type *types[MAX_ITEMLIST+2];
with this:
types[counter] = i_ptr;
And that's declared at the beginning of the function so it wouldn't go out of scope until the function ends. (you're probably right, I'm just trying to understand it)

EDIT: Thanks Derakon,
I moved

					object_type *i_ptr;
					object_type object_type_body;

to the top of the function and it seems to be fixed now, although I still don't really understand why..

[buzzkill]

So, just curious, will known flavors show up as mimics (or is that too evil). Will perception allow you to recognize a mimic as you approach it?

You see a Potion of Healing. It casts a nether bolt. You feel your life draining away. <more>
You Die.

[Derakon]

If the fake object goes out of scope, then why don't the real objects go out of scope? It looks to me like both the fake objects and the real objects are assigned to
object_type *types[MAX_ITEMLIST+2];
with this:
types[counter] = i_ptr;
And that's declared at the beginning of the function so it wouldn't go out of scope until the function ends. (you're probably right, I'm just trying to understand it)

The real objects were allocated memory on the heap, not the stack. The heap is free-floating memory and doesn't care about scoping rules; however, because of this, it's very easy to create memory leaks (where objects are allocated to the heap, then become irrelevant to the program without first getting deallocated). When you do

object_type object_type_body;

you're creating an object_type on the stack; it will still go out of scope when the function finishes at the very least. In order to create the object on the heap, you'd need to do something like this:

object_type* object_type_body;
object_type_body = (object_type*) malloc(sizeof(object_type));

NOTE: it's been ages since I did any C coding, so this is probably wrong somehow. But the general idea is sound: create a pointer of the appropriate type, ask for a block of memory on the heap with the same size as the type you need to point to, then fill that memory with data.

Just remember to free the memory when you're done with it!

[PowerDiver]

If the fake object goes out of scope, then why don't the real objects go out of scope? It looks to me like both the fake objects and the real objects are assigned to
object_type *types[MAX_ITEMLIST+2];
with this:
types[counter] = i_ptr;
And that's declared at the beginning of the function so it wouldn't go out of scope until the function ends. (you're probably right, I'm just trying to understand it)

EDIT: Thanks Derakon,
I moved

					object_type *i_ptr;
					object_type object_type_body;

to the top of the function and it seems to be fixed now, although I still don't really understand why..

I didn't look hard enough to understand your code completely, but that is worrisome. I would guess that if you have more than one type of mimic you will still have a bug. You have only one object_type_body. If you use it twice in the function, the previous value will be overwritten, and the i_ptr you copied into your list will no longer point to what you think it should be pointing to.

Mallocing and freeing leads to nightmares. I would suggest that you keep a suitably large array of object_type_body. The function starts by setting the counter to 0, and increment it each time you need a new one, checking of course that you don't go past the end of the array.

I am also unclear on who ever looks at the list that you put i_ptr into. If anyone ever looks at it after this function exits, then you have to declare the body array outside of the function.

[zaimoni]

EDIT: Thanks Derakon,
I moved

object_type *i_ptr;
object_type object_type_body;

to the top of the function and it seems to be fixed now, although I still don't really understand why..

I didn't look hard enough to understand your code completely, but that is worrisome.

It's all over the V codebase he forked from, and highly annoying (I'd prefer to ditch the i_ptr/o_ptr for this construct and just use the reference operator & whenever needed).

[will_asher]

First, of all, I just tested and Powerdiver is right, it messes up when there's more than one mimmic (but at least it doesn't crash).
Second, I still don't understand why it forgets the fake objects. What I did to create the fake objects is exactly the same as the way the game creates real objects except that the object isn't actually placed in the dungeon.
Third, if you tell me to copy the way something was done somewhere else in the code and make certain change(s) in it to do what I want to do, then I'll be fine. But if you tell me to keep a suitably large array of object_type_body, then I don't know where to start.

[PowerDiver]

The problem is that you need a separate body for each object. The body is where all of the data about the properties is stored. When you have only one, the second time you process a mimic you overwrite the values of the first mimic.

At the top of your function, you should have something like

#define MAX_MIMICS_PER_LEVEL 200
object_type mimic_object_type_body[MAX_MIMICS_PER_LEVEL];
int num_mimics = 0;

I picked 200 beause 100 might not be not enough if you allow for NPP style coin pits. Some would say the define belongs somewhere else, in some include file. It certainly does if you modify the monster generation code to guarantee not to generate more mimics than that value.

To be fully correct, you would want a value different from 200. I don't know if there is a MAX_MONSTERS definition used elsewhere. You could use DUNGEON_HEIGHT * DUNGEON_WIDTH or whatever they are called. It's slightly gross to use that much space when you know you will have at most 10 mimics the vast majority of the time. Since the code is not critical, you could just pick a value that is typically not reached.

Then, in the code where you need a new mimic object,
i_ptr = &mimic_object_type_body[num_mimics];
if (num_mimics < MAX_MIMICS_PER_LEVEL - 1) num_mimics++;

This is equivalent to what you are currently doing, with MAX_MIMICS_PER_LEVEL being 1. If you have too many mimics for whatever value you choose, you will see the same kind of behavior that you currently see for more than one mimic.

[will_asher]

The maximum number of monsters is 1024 set in limits.txt and in the code as
z_info->m_max
...but I think 200 will be more than sufficient. (Even if I had creeping coin pits, it doesn't show coins on the object list. And currently, I don't have CHAR_MULTI on creeping coins, I still deciding whether I want that.)
Thanks, it looks like it's working just fine now.