BugID: 604 (John Adams) SpawnSet Heading broken

[John Adams]

Bug ID: 604 - SpawnSet Heading broken
Bug Date: 2012-07-16 20:14:44
Priority: Medium
Originated From World: EQ2Emu DB Project (906)
Category: Interface
Sub-Category: User Interface
Causes a Crash: Affects gameplay
Reproducible: Always Happens
Version: SOEBuild=7628L

Details:
Trying to set 1 gobbie spawn to face the wall, seems it's changing every spawns placement heading instead of just the one placement_id.
Spawn: a Grungetalon gemcutter (990015), Zone: Kaladim

[John Adams]

[22:52] <@Jabantiz> heading = (heading - 180) * 64;
[22:52] <@Jabantiz> no idea what that is for but would explain the /spawn set head 180 resetting to 0
[22:53] <@Jabantiz> and i don't think john's bug is going to be an easy fix

Jab, you got plenty of other stuff on your plate. Don't worry about this one, I'll take a look and yell when I need help

This bug didn't used to happen. I know for a fact because I used the crap out of /spawn set heading for years. Has to be something new that's changed, so I will look directly at the code and if I don't see it, I will revert til I find where it broke.


Edit: What changed might be that Parser now parses spawns "consolidated", meaning the same spawn ID has different randomized appearances based on how many differences it detects in the raw packets. Previously, there was 1 spawn_id per appearance. Perhaps this bug has just surfaced because of this.

Regardless, the changes to any targeted spawn should only happen to the spawn targeted, and not every spawn that matches that ID in the DB. This is going to be a doosey!

[John Adams]

Okay, just talking this out to myself. No one needs to get involved yet

When you /spawn set heading, here's what happens -

In Command.cpp, first:

Code: Select all

spawn_set_values["heading"] = SPAWN_SET_VALUE_HEADING;

So that array element now = 29.

Next, the switch:

Code: Select all

			case SPAWN_SET_VALUE_HEADING:{
				sprintf(tmp, "%f", target->GetHeading());
				target->SetHeading(atof(value) + 360, send_update);
				break;

This sets the value that is going to be updated in the Database. Not concerned about the Math right now, just how this gets written.


Next, the 2nd switch:

Code: Select all

			case SPAWN_SET_VALUE_HEADING:{
				target->SetHeading(atof(value), send_update);
				target->SetSpawnOrigHeading(target->GetHeading());
				break;

This is where the spawn in the world changes heading visually, and I believe ALL SPAWNS with this guys spawns.id value are swiveling to the new heading - in game, only (theoretically).


Then, the command gets processed and this code gets called in COMMAND_SPAWN_SET.
This is the critical part. What gets written to the DB?

Code: Select all

if((set_type >= SPAWN_SET_VALUE_RESPAWN && set_type <=SPAWN_SET_VALUE_LOCATION) || (set_type >= SPAWN_SET_VALUE_EXPIRE && set_type <=SPAWN_SET_VALUE_Z_OFFSET))
{
	if(spawn->GetSpawnLocationID() > 0 && database.UpdateSpawnLocationSpawns(spawn))

which basically checks if the constant value is between certain ranges, then update the location (we should re-arrange these so they are not all over the map someday). In this case, SPAWN_SET_VALUE_HEADING == 29, and is between 25 and 30 (the first check). If so, save the updated location.


Okay, let's update the location information now -

Code: Select all

bool WorldDatabase::UpdateSpawnLocationSpawns(Spawn* spawn){
	Query query;
	query.RunQuery2(Q_UPDATE, "update spawn_location_placement set x=%f, y=%f, z=%f, heading=%f, x_offset=%f, y_offset=%f, z_offset=%f, respawn=%lu, expire_timer=%lu, expire_offset=%lu, grid_id=%lu where id = %lu",
		spawn->GetX(), spawn->GetY(), spawn->GetZ(), spawn->GetHeading(), spawn->GetXOffset(), spawn->GetYOffset(), spawn->GetZOffset(), spawn->GetRespawnTime(), spawn->GetExpireTime(), spawn->GetExpireOffsetTime(), spawn->appearance.pos.grid_id, spawn->GetSpawnLocationPlacementID());
	if(query.GetErrorNumber() && query.GetError() && query.GetErrorNumber() < 0xFFFFFFFF){
		LogWrite(WORLD__ERROR, "World", "Error in UpdateSpawnLocationSpawns query '%s': %s", query.GetQuery(), query.GetError());
		return false;
	}
	return true;
}

Because the WHERE clause specifies only the `id` column in the `spawn_location_placement` table, there should be no way that more than this 1 placement is getting updated in the database (spawn->GetSpawnLocationPlacementID()). Even if visually in-world, they are all flipping around, I theorize that's just a visual bug with how spawn.appearances get updated in the world.


If this is true, this is not critical then, though I would like to see it fixed eventually (to save anymore confusion). To fix this, we'd have to limit appearance updates to the current placement ID and not spawn_id for anything location-based.


Thoughts?


I'll test this theory out later when I get home. No code changes, just observing DB updates and /reload spawn after setting 1 spawn that affects many.

[Jabantiz]

I tried the /reload spawns last night and it works exactly like you describe, only the targeted mob saves the new heading the rest revert back to what they were.

[John Adams]

Good, so it's settled. Visual bug only. I'll lower the priority, and we'll have to consider how to apply "appearance" updates on-the-fly to spawn ID's in specific spawn locations, vs all IDs. Someday.

Thanks for checking it out before I wasted my time

[Jabantiz]

This is fixed now as well as the 180 issue.

When you do a /spawn set command it calls a function in zoneserver that then calls the command, minus the save part, for all other spawns in the zone with the same DB ID. I just added a check to see if it was a location change (x, y, z, heading) and return out of the function so it doesn't changes other spawns in the zone. This is currently on Dev SVN.