Possible issue using GetItem() in SharedDatabase::SetStartingItems()

[Drajor]

Heyas,

I add some items to players inventories during character creation and everything works fine except for an item that has a click effect on it.

In the SharedDatabase::SetStartingItems() method I add a charm for each class like this.

	// Charms
	if (pp->class_ == WARRIOR)
		inv->PutItem(inv->FindFreeSlot(0,0), GetItem(3400));

The item appears in the player inventory correctly, however when I try to right click the item I get the message 'Item is out of charges'. If I delete the item and #si it again, the click effect works fine.

This makes me think that something is not getting initialised correctly when calling GetItem() at this point. Any ideas? I looked at the code but I can't find anything which would cause this.

EDIT: I am using HoT client and rev 2214 of the source.

[c0ncrete]

this doesn't address the issue you're having, but why not just edit the starting_items table in the database?

[Drajor]

Because it is /much/ easier (for me) to change 10 lines of code in the source. The starting_items table offers flexibility I frankly don't need.

[c0ncrete]

Originally Posted by Drajor

Heyas,

I add some items to players inventories during character creation and everything works fine except for an item that has a click effect on it.

In the SharedDatabase::SetStartingItems() method I add a charm for each class like this.

    // Charms
    if (pp->class_ == WARRIOR)
        inv->PutItem(inv->FindFreeSlot(0,0), GetItem(3400));

The item appears in the player inventory correctly, however when I try to right click the item I get the message 'Item is out of charges'. If I delete the item and #si it again, the click effect works fine.

This makes me think that something is not getting initialised correctly when calling GetItem() at this point. Any ideas? I looked at the code but I can't find anything which would cause this.

EDIT: I am using HoT client and rev 2214 of the source.

in SetStartingItems(), it looks as though CreateBaseItem() is called on the result of GetItem(), and then that item instance is passed to inv->PutItem().

[Drajor]

I changed my code to use CreateBaseItem() however the result is still the same. I thought you had it solved c0ncrete! Thank you

EDIT: New code FYI:

	// Class Charms.
	const int BASE_CHARM = 3400;
	const int NUM_CHARM_RANKS = 100;
	const Item_Struct* classCharm =  GetItem(BASE_CHARM + ((pp->class_-1) * NUM_CHARM_RANKS));
	inv->PutItem(inv->FindFreeSlot(0,0), *CreateBaseItem(classCharm, 0));

[c0ncrete]

that looks like exactly what SetStartingItems() is doing now. i'm at a loss.

[lerxst2112]

What is MaxCharges set to on the item?

Did you try putting the item in the starting_items table? Did it work?

Honestly, I can't imagine a situation where having to recompile the code to change the items is easier than just adding them to the database, but do what you like.

[Drajor]

Hi lerxst2112,

maxcharges is -1 in the db. I cant put items into the starting_items table because it is *gone*, deleted. As I stated, I do not need the flexibility offered by the starting_items table, I do not change what items players start with now that they are set.

The concept of putting starting_items into a table is just a design choice, probably to make customisation easier for people not comfortable changing the source. The devs just chose to put starting items in a table and decided to hardcode starting stats yet both can be customised.

[Tabasco]

*CreateBaseItem(classCharm, 0));

You're Telling CreateBaseItem to create an item instance with 0 charges.

As an aside, using the starting_items table is good design. You may not need the flexibility, but what about the convenience?
Once you have your build environment it's pretty easy to recompile but using the table in the database is going to be faster to do in any case, fewer keystrokes, and less opportunity for errors that spawn support threads. In terms of time alone you're at a significant net loss.

[Drajor]

Originally Posted by Tabasco

You're Telling CreateBaseItem to create an item instance with 0 charges.

As an aside, using the starting_items table is good design. You may not need the flexibility, but what about the convenience?
Once you have your build environment it's pretty easy to recompile but using the table in the database is going to be faster to do in any case, fewer keystrokes, and less opportunity for errors that spawn support threads. In terms of time alone you're at a significant net loss.

ItemInst* SharedDatabase::CreateBaseItem(const Item_Struct* item, sint16 charges) {
	ItemInst* inst = NULL;
	if (item) {
		if (charges == 0)
			charges = item->MaxCharges;

		if(item->CharmFileID != 0 || (item->LoreGroup >= 1000 && item->LoreGroup != -1)) {
			inst = new EvoItemInst(item, charges);
			((EvoItemInst*)inst)->Initialize(this);
		}
		else 
			inst = new ItemInst(item, charges);		
	}
	return inst;
}

Where parameter charges is 0, it presumably uses the value maxcharges from the DB.

[Tabasco]

You're right, I didn't dig into it deep enough, but in every other case in SetStartingItems charges is set, so it might be worth a try to specify your charges.

You could also revert and just do this:

SET SQL_MODE="NO_AUTO_VALUE_ON_ZERO";

CREATE TABLE IF NOT EXISTS `starting_items` (
  `id` int(11) unsigned NOT NULL AUTO_INCREMENT,
  `race` int(11) NOT NULL DEFAULT '0',
  `class` int(11) NOT NULL DEFAULT '0',
  `deityid` int(11) NOT NULL DEFAULT '0',
  `zoneid` int(11) NOT NULL DEFAULT '0',
  `itemid` int(11) NOT NULL DEFAULT '0',
  `item_charges` tinyint(3) unsigned NOT NULL DEFAULT '1',
  `gm` tinyint(1) NOT NULL DEFAULT '0',
  `slot` mediumint(9) NOT NULL DEFAULT '-1',
  PRIMARY KEY (`id`,`race`)
) ENGINE=InnoDB  DEFAULT CHARSET=latin1 AUTO_INCREMENT=1 ;

INSERT INTO `starting_items` (`id`, `race`, `class`, `deityid`, `zoneid`, `itemid`, `item_charges`, `gm`, `slot`) VALUES
('', 0, 0, 0, 0, <itemid for stuff everyone gets>, 1, 0, -1),
('', 0, 1, 0, 0, <itemid for stuff warriors get>, 1, 0, -1),
etc;

[Drajor]

FYI I downloaded the rev2214 binaries again and the rev2214 DB, made a new DB and set it all up cleanly. I add JBoots to starting_items, create a new toon, right click the boots and get 'Item is out of charges'.

[Drajor]

Originally Posted by Tabasco

You're right, I didn't dig into it deep enough, but in every other case in SetStartingItems charges is set, so it might be worth a try to specify your charges.

You could also revert and just do this:

SET SQL_MODE="NO_AUTO_VALUE_ON_ZERO";

CREATE TABLE IF NOT EXISTS `starting_items` (
  `id` int(11) unsigned NOT NULL AUTO_INCREMENT,
  `race` int(11) NOT NULL DEFAULT '0',
  `class` int(11) NOT NULL DEFAULT '0',
  `deityid` int(11) NOT NULL DEFAULT '0',
  `zoneid` int(11) NOT NULL DEFAULT '0',
  `itemid` int(11) NOT NULL DEFAULT '0',
  `item_charges` tinyint(3) unsigned NOT NULL DEFAULT '1',
  `gm` tinyint(1) NOT NULL DEFAULT '0',
  `slot` mediumint(9) NOT NULL DEFAULT '-1',
  PRIMARY KEY (`id`,`race`)
) ENGINE=InnoDB  DEFAULT CHARSET=latin1 AUTO_INCREMENT=1 ;

INSERT INTO `starting_items` (`id`, `race`, `class`, `deityid`, `zoneid`, `itemid`, `item_charges`, `gm`, `slot`) VALUES
('', 0, 0, 0, 0, <itemid for stuff everyone gets>, 1, 0, -1),
('', 0, 1, 0, 0, <itemid for stuff warriors get>, 1, 0, -1),
etc;

The charges field in starting_items is type tinyint, if I wanted to start with a clicky item I would have to set it to zero anyway. I tried -1 as the parameter before I posted earlier. I have to say there seems to be an overriding assumption here that * I am wrong * and I should just do it some other way. If I find a bug good, I will save some folks some time in the future, if it is my error, even better, I learned something new.

[sorvani]

Originally Posted by Drajor

EDIT: I am using HoT client and rev 2214 of the source.

There is no HoT client. There is Titanium (dics), SoF (discs,unfortunately), SoD (Steam), and UF (Steam) among the stable builds.

[Drajor]

Originally Posted by sorvani

There is no HoT client. There is Titanium (dics), SoF (discs,unfortunately), SoD (Steam), and UF (Steam) among the stable builds.

Sorry, I am using UF.