malloy@nprdc.arpa (Sean Malloy) (08/11/88)
I've gotten a message saying that my explanation of what to dike out of the
inventory bug patch was confusing, so I'm reposting a copy with the excess
information already diked out.
This patch fixes the bug that causes objects in your inventory to lose
their descriptions, i.e., the 'blessed +1' bug.
Sean Malloy
Navy Personnel Research & Development Center
San Diego, CA 92152-6800
malloy@nprdc.arpa
*******************************
*** oitem.c.old Wed Aug 10 08:58:51 1988
--- oitem.c Wed Aug 10 08:46:25 1988
***************
*** 39,49 ****
--- 39,62 ----
return(new);
}
+ void get_obj(new,id)
+ pob new;
+ int id;
+ {
+ *new = Objects[id];
+ new->objstr = salloc(Objects[id].objstr);
+ new->cursestr = salloc(Objects[id].cursestr);
+ new->truename = salloc(Objects[id].truename);
+ }
+
void make_cash(new,level)
pob new;
int level;
{
+ /*****
*new = Objects[CASHID];
+ *****/
+ get_obj(new,CASHID);
new->basevalue = random_range(level*level+10)+1; /* aux is AU value */
new->objstr = salloc(cashstr());
new->cursestr = new->truename = new->objstr;
***************
*** 54,60 ****
--- 67,76 ----
int id;
{
if (id == -1) id = random_range(NUMFOODS);
+ /*****
*new = Objects[FOODID+id];
+ *****/
+ get_obj(new,FOODID+id);
}
***************
*** 62,68 ****
--- 78,87 ----
pob new;
struct monster *m;
{
+ /*****
*new = Objects[CORPSEID];
+ *****/
+ get_obj(new,CORPSEID);
new->charge = m->id;
new->weight = m->corpseweight;
new->basevalue = m->corpsevalue;
***************
*** 133,139 ****
--- 152,161 ----
int id;
{
if (id == -1) id = random_range(NUMRINGS);
+ /*****
*new = Objects[RINGID+id];
+ *****/
+ get_obj(new,RINGID+id);
if (new->blessing == 0) new->blessing = itemblessing();
if (new->plus == 0) new->plus = itemplus()+1;
if (new->blessing < 0) new->plus = -1 - abs(new->plus);
***************
*** 144,150 ****
--- 166,175 ----
int id;
{
if (id == -1) id = random_range(NUMTHINGS);
+ /*****
*new = Objects[THINGID+id];
+ *****/
+ get_obj(new,THINGID+id);
if (strcmp(new->objstr,"grot") == 0) {
new->objstr = salloc(grotname());
new->truename = new->cursestr = new->objstr;
***************
*** 157,163 ****
--- 182,191 ----
int id;
{
if (id == -1) id = random_range(NUMSCROLLS);
+ /*****
*new = Objects[SCROLLID+id];
+ *****/
+ get_obj(new,SCROLLID+id);
/* if a scroll of spells, aux is the spell id in Spells */
if (new->id == SCROLLID+1) {
new->aux = random_range(NUMSPELLS);
***************
*** 169,175 ****
--- 197,206 ----
int id;
{
if (id == -1) id = random_range(NUMPOTIONS);
+ /*****
*new = Objects[POTIONID+id];
+ *****/
+ get_obj(new,POTIONID+id);
if (new->plus == 0) new->plus = itemplus();
}
***************
*** 178,184 ****
--- 209,218 ----
int id;
{
if (id == -1) id = random_range(NUMWEAPONS);
+ /*****
*new = Objects[WEAPONID+id];
+ *****/
+ get_obj(new,WEAPONID+id);
if ((id == 28) || (id == 29)) /* bolt or arrow */
new->number = random_range(20)+1;
if (new->blessing == 0) new->blessing = itemblessing();
***************
*** 196,202 ****
--- 230,239 ----
int id;
{
if (id == -1) id = random_range(NUMSHIELDS);
+ /*****
*new = Objects[SHIELDID+id];
+ *****/
+ get_obj(new,SHIELDID+id);
if (new->plus == 0)
new->plus = itemplus();
if (new->blessing == 0) new->blessing = itemblessing();
***************
*** 211,217 ****
--- 248,257 ----
int id;
{
if (id == -1) id = random_range(NUMARMOR);
+ /*****
*new = Objects[ARMORID+id];
+ *****/
+ get_obj(new,ARMORID+id);
if (new->plus == 0) new->plus = itemplus();
if (new->blessing == 0) new->blessing = itemblessing();
if (new->blessing < 0)
***************
*** 226,232 ****
--- 266,275 ----
{
if (id == -1) id = random_range(NUMCLOAKS);
Objects[CLOAKID+4].plus = 2;
+ /*****
*new = Objects[CLOAKID+id];
+ *****/
+ get_obj(new,CLOAKID+id);
if (new->blessing == 0) new->blessing = itemblessing();
}
***************
*** 235,241 ****
--- 278,287 ----
int id;
{
if (id == -1) id = random_range(NUMBOOTS);
+ /*****
*new = Objects[BOOTID+id];
+ *****/
+ get_obj(new,BOOTID+id);
if (new->blessing == 0) new->blessing = itemblessing();
}
***************
*** 244,250 ****
--- 290,299 ----
int id;
{
if (id == -1) id = random_range(NUMSTICKS);
+ /*****
*new = Objects[STICKID+id];
+ *****/
+ get_obj(new,STICKID+id);
new->charge = itemcharge();
if (new->blessing == 0) new->blessing = itemblessing();
}
***************
*** 253,259 ****
--- 302,311 ----
pob new;
{
if (id == -1) id = random_range(NUMARTIFACTS);
+ /*****
*new = Objects[ARTIFACTID+id];
+ *****/
+ get_obj(new,ARTIFACTID+id);
}
***********************
*** osave.c.old Wed Aug 10 09:02:55 1988
--- osave.c Wed Aug 10 08:35:42 1988
***************
*** 246,251 ****
--- 246,256 ----
fprintf(fd,"Null Object. Report if you see this!\n");
}
else {
+ /*****/
+ o->objstr = salloc(Objects[o->id].objstr);
+ o->cursestr = salloc(Objects[o->id].cursestr);
+ o->truename = salloc(Objects[o->id].truename);
+ /*****/
fwrite((char *)o,sizeof(objtype),1,fd);
fprintf(fd,"%s\n",o->truename);
}cjs@bruce.oz (Chris Stuart) (08/17/88)
From article <749@james.nprdc.arpa>, by malloy@nprdc.arpa (Sean Malloy): > > This patch fixes the bug that causes objects in your inventory to lose > their descriptions, i.e., the 'blessed +1' bug. > > [ Large number of patches to lots of files follow. ] > I also found the same problem that Sean has found, although my method of solving the problem was a little different. I also posted a solution, along with lots of other bug patches, and am not sure if it got out of Australia. Anyway, with regard to the vanishing names problem. The difficulty is that strings (names) are kept in 'malloc'ed memory, and that there are many occassions were object structures are copied, leaving different objects with pointers to the same string. Under what circumstances should the string be freed? Sean's solution is to ensure that every copy of an object structure also copies the strings, so that strings always have only one pointer pointing to them, and may be safely 'free'ed when the object is 'free'ed. My solution was never to free strings. On the face of it, Sean has the right idea. However, i would point out both methods have problems. For Sean, the difficulty comes from the fact that there are many places where object structures are freed without freeing the strings, and several places where pointers to an object structure are lost and the space never recovered. Thus one effect of all the extra work to allocate extra strings is that MORE memory is lost and never recovered under these circumstances. My method also loses memory permanently, because space allocated to strings is never freed. But freeing space is dangerous anyway, because there is no consistent method of checking in C if freed space really has no pointers to it. Really we need a language with a garbage collector, like LISP. Also, in the omega source, monster strings are NEVER freed, and object strings only OCCASSIONALLY freed, and then only in ONE function. So for any easy fix, delete the code that frees strings. Here is the patch, to be made in oinv.c. *** oinv.orig Wed Aug 17 11:00:27 1988 --- oinv.c Wed Aug 17 11:00:39 1988 *************** *** 388,394 int n; pob obj; { ! int i,conformed=FALSE,subtracted=FALSE,freetrue,freecurse; if (obj != NULL) for(i=0;i<MAXITEMS;i++) if (Player.possessions[i] == obj) { if (! subtracted) { --- 388,394 ----- int n; pob obj; { ! int i,conformed=FALSE,subtracted=FALSE; if (obj != NULL) for(i=0;i<MAXITEMS;i++) if (Player.possessions[i] == obj) { if (! subtracted) { *************** *** 404,415 } } if (obj->number < 1) { - freetrue=(obj->objstr != obj->truename); - freecurse=((obj->objstr != obj->cursestr)&& - (obj->cursestr != obj->truename)); - free(obj->objstr); - if (freetrue) free(obj->truename); - if (freecurse) free(obj->cursestr); free((char *) obj); } } --- 404,409 ----- } } if (obj->number < 1) { free((char *) obj); } } Then, if you want to be careful about preserving memory, look at every call to salloc in the code, and see if you can fix it so that the string pointers simply point to exactly the same part of memory. Certainly, all the functions in oitem.c like potionname and scrollname could return pointers to constant strings rather than use the global buffer Str4 (both methods are bad programming pratice, so why worry) and then all object name could point directly to those strings. This would save a LOT of memory space, and take much less string handling overhead. But the simple patch i have given is a safe and sure way of making sure the missing names problem goes away, with less loss of memory than you might think at first. Comments welcome, and do look at the other bug fixes i have posted. Chris Stuart
bph@buengc.BU.EDU (Blair P. Houghton) (08/19/88)
In article <498@bruce.oz> cjs@bruce.oz (Chris Stuart) writes: >From article <749@james.nprdc.arpa>, by malloy@nprdc.arpa (Sean Malloy): >> >> This patch fixes the bug that causes objects in your inventory to lose >> their descriptions, i.e., the 'blessed +1' bug. [...] >return pointers to constant >strings rather than use the global buffer Str4 (both methods are bad >programming pratice, so why worry) and then all object name could point >directly to those strings. This would save a LOT of memory space, and take >much less string handling overhead. "...both methods are *bad* programming practice?" I don't know if I ever would do a string copy in a situation like this. Using the constant strings exclusively and only bothering with the pointers seems like good practice, to me; then again, I tend to revert to assembly-style programming when I don't have my "higher- level language" wings on :-). Anyway, it seems that problem has just disappeared when I play the game. I think I'll go try to break it and see exactly where it comes up... --Blair