RE: [Adonthell-devel] Equipment test

[Mike Avery]

>   As we agreed, a whole stack of items can be equipped (in case of
> arrows and the like). That means, whenever equipping items, one has to
> pass the count. However, that leads to one problem. When there is
> already a stack equipped, how do you return it's size? After all,
> manager::equip returns an item_base, but the count is kept 
> with the slot
> class. So I chose to use an inout parameter to return the count of the
> returned item. Same is true for manager::unequip.

Cool.


> * moving items between equipment slots
> 
>   In that case, we must not forget to call item::unequipped. However,
> how can the manager tell whether a given item sits in an 
> equipment or a
> normal slot? One way was adding a flag to the slot class (Equipment),
> that contains exactly this information.
> This has another advantage: we can now ensure that manager::equip does
> not accept slots that aren't meant for equipment.

Great idea.
 
> * adding items of the same kind
> 
>   For example, I have a slot with 10 items and want to add 
> another 10 of
> the same sort. In that case, the currently equipped items 
> should not be
> removed. Instead the given items should simply be added to 
> the slot. In
> the case that not all of them fit (because of stack size restriction),
> the number of those remaining will be returned via the count 
> parameter.

Perfect!
 
> Issues:
> =======
> 
> A problem that is quite severe is related to SWIG. As Python 
> python does
> not inout parameters, the Python wrappers of manager::equip/unequip
> return a tuple instead of a single value. However, if the return value
> of the method is NULL, then SWIG will discard it


> Do you think I should report this issue to the SWIG people 
> and hope that
> they change this behaviour? Or do you have another idea. (Using SWIG
> 1.3.19 here, btw.)
 
I think it should be reported yeah.  For our purposes though, what about a
slight change to work around this?

Can we work around this by using a count of 0 as the "null count" instead of
NULL?  They're identical for our uses unless there's something that I have
overlooked?
 
 
> Another possible problem exists in slot::get_item, which isn't free of
> side-effects. It does assign the slot to immutable items (which cannot
> have a persistant slot assigned). That's what we want in most cases,
> however, sometimes this shouldn't happen. Guess an optional parameter
> could take care of that.
 
You mean coins or whatever I assume, yeah.
 
> Questions left:
> ===============
> 
> I'm not exactly sure when to call item::equipped / unequipped. Right
> now, unequipped is only called if the last item has been 
> removed. Should
> something similar be done with equipped: i.e. only call it when a new
> item is equipped, but not when an item is added to an existing stack?

I think this should be the last step in every transaction involving moving
equipment on/off of characters. It gives us plot hooks based on un/equipping
of certain items.

Should the un/equip methods examine the in/out items and make effect
adjustments themselves, or should all un/equip transactions call
un/equipped() upon success and we can handle applying/removing effects
there?

This raises the question then of whether un/equipped() should be moved from
the item to the manager class.

manager::unequiped( removed_item )  //negate removed item effects, apply
plot hooks
manager::equiped ( item ) // apply equiped item effects, apply plot hooks

If we want un/equip() to apply effects, then manager::un/equipped() doesn't
need to exist, and item::un/equipped() is good for plot hooks, but I don't
see much use for it beyond that.



> Bugs fixed:
> ===========
> 
> I believe that I found a bug in py_adonthell.i:
> 
> The string returned by '%typemap(python,out) string &' shouldn't be
> deleted. After all, it's most likely a member of some class, 
> which will
> still need it.
> 
> Without that fix, my test script had segfaulted, because when calling
> slot::id (), the id would be returned and also deleted(!).

Igh.  I hope this didn't drive you too nuts :)

I'm going to start work on the manager class character data structures.


Mike