World fix and fireball test #195
Conversation
Source/ACE/Entity/PhysicsData.cs
Outdated
| public Position Velocity; | ||
| public Position Omega; // rotation | ||
| public AceVector3 Velocity; | ||
| public AceVector3 Omega; // rotation |
There was a problem hiding this comment.
In PhysicsData.cs line 51, please consider if the constructor needs to have some base values:
public PhysicsData()
{
Velocity = new AceVector3(0.0f, 0.0f, 0.0f);
Omega = new AceVector3(0.0f, 0.0f, 0.0f);
}
It is worth checking because I had been getting a crash where Velocity was null, when a character was logging into the server.
Source/ACE/Entity/SpellEntity.cs
Outdated
| /// <summary> | ||
| /// Spell Entity | ||
| /// </summary> | ||
| public class SpellEntity : MutableWorldObject |
There was a problem hiding this comment.
This needs a better name. Is there a particular reason a spell gets an entire class definition, though? The only difference I see here is that it has a PlayScript implementation that creates an Effect. There's nothing about the class definition that tells me it's a spell. I think this could have been done in MutableWorldObject as is, with all the Physics values being passed into a constructor, and then checking the PhysicsData.PhysicsState in the PlayScript method.
Also, I recall seeing a video of multiple effects. Was that not yours? Is that implemented here?
There was a problem hiding this comment.
I wanted to keep it separate because I am not sure yet what all it will needed,
But PlayScript may have to be far more advanced as we learn more.
I don't want to put all that logic in all mutable objects, I think it would create a mess as things progress. for now I thinks its best to keep them separated.
And if we find other objects with different code then PlayScript, we can then make it more generic if we keep them separated. Code will be a little bigger but much easier to read in the long run as we have different kinds of Entities.
| switch (templateId) | ||
| { | ||
| case 0: | ||
| wo.SpellID = Spell.FlameBolt; |
There was a problem hiding this comment.
yeah, as I said above, these values should be pushed into the MutableWorldObject constructor instead of being replaced after instantiation.
| @@ -19,7 +19,7 @@ public abstract class WorldObject | |||
| /// <summary> | |||
| /// wcid - stands for weenie class id | |||
| /// </summary> | |||
| public ushort WeenieClassid { get; protected set; } | |||
| public ushort WeenieClassid { get; set; } | |||
There was a problem hiding this comment.
I told you repeatedly to not do this.
| { | ||
|
|
||
| ushort weenieClassId = 0; | ||
| MutableWorldObject mobj = new MutableWorldObject(ObjectType.MissileWeapon, new ObjectGuid(CommonObjectFactory.DynamicObjectId, GuidType.None), "Spell", weenieClassId, ObjectDescriptionFlag.None, WeenieHeaderFlag.Spell, position); |
There was a problem hiding this comment.
pull your switch block above this to find out what weenieClassId you need and pass it in. We discussed this several times.
Clean up - Synced with Latest Prs