Moved Identify to WorldObject #473
Conversation
There was a problem hiding this comment.
The purpose of moving the Identify response functionality into the WorldObject is to use inheritance to break down the different behaviors of identify.
Consequently, there are a lot of parts of the identify packet that should be handled by overriding the virtual method in subclasses. Some of those subclasses don't exist yet (weapon, armor), but others do (creature). For the classes that do exist, you should handle this functionality appropriately, and for the classes that don't you should at minimum place a TODO in the code.
Source/ACE/Entity/WorldObject.cs
Outdated
| // TODO: There are probably other checks that need to be made here | ||
| if (ItemType == ItemType.Creature && GetType().Name != "DebugObject") | ||
| { | ||
| WriteIdentifyObjectCreatureProfile(writer, (Creature)this); |
There was a problem hiding this comment.
This should be handled in the "Creature" class via virtual method override, not here, as it is a creature exclusive profile.
Source/ACE/Entity/WorldObject.cs
Outdated
| WriteIdentifyObjectStringsProperties(writer, flags, propertiesString); | ||
| WriteIdentifyObjectDidProperties(writer, flags, propertiesDid); | ||
| WriteIdentifyObjectSpellIdProperties(writer, flags, propertiesSpellId); | ||
| WriteIdentifyObjectArmorProfile(writer, flags, propertiesArmor); |
There was a problem hiding this comment.
Eventually this should probably be moved to an "Armor" subclass
Source/ACE/Entity/WorldObject.cs
Outdated
| { | ||
| WriteIdentifyObjectCreatureProfile(writer, (Creature)this); | ||
| } | ||
| WriteIdentifyObjectWeaponsProfile(writer, flags, propertiesWeaponsD, propertiesWeaponsI); |
There was a problem hiding this comment.
Similar to "Armor", this will need to go into a "Weapon" subclass.
|
@ddevec This closer to what you're expecting? It will break most of the creature assessment panels as 99% of the objects in the world are not Creatures yet. |
Source/ACE/Entity/Creature.cs
Outdated
| @@ -495,5 +497,162 @@ protected virtual void UpdateVitalInternal(CreatureVital vital, uint newVal) | |||
| new ActionChain(this, () => VitalTickInternal(vital)).EnqueueChain(); | |||
| } | |||
| } | |||
|
|
|||
| public override void SerializeIdentifyObjectResponse(BinaryWriter writer) | |||
There was a problem hiding this comment.
This has a signficiant amount of duplicated code with WorldObject's SerializeIdentifyObjectResponse. Consider calling "base.SerializeIdentifyObjectResponse", or if that is inappropriate, refactoring such that you deduplicate the code between the two calls.
There was a problem hiding this comment.
I did no refactoring, just trying to see if this is generally what you were saying
There was a problem hiding this comment.
I think this is generally right then.
If breaking creature identification for non-creatures DebugObjects is a problem, we may consider not doing this refactor yet.
I would argue that there is little point returning a proper identification for an improperly implemented underlying type, but I'm not working with general WorldObject properties or their uses, and I have a limited view in this realm.
There was a problem hiding this comment.
to be fair, most of the stuff returned in the creatureprofile displayed in the client are bad numbers/stats, so i'm kinda okay with breaking it in one regard..
i've got some thoughts on how to implement weenietype and delete debugobject going forward but it'll take some db work + thinking/coding time to propose properly, possibly looking to do that in a week or so
There was a problem hiding this comment.
in short, the way we key object creation is bad and unsustainable. ObjectDescriptionFlags is more mutable than we originally suspected.
Source/ACE/Entity/Creature.cs
Outdated
| flags |= IdentifyResponseFlags.ArmorProfile; | ||
| } | ||
|
|
||
| // Weapons Profile |
There was a problem hiding this comment.
Why do creatures deal with weapons and armor profiles?
No description provided.