Add Cow object #482

LtRipley36706 · 2017-07-31T06:44:19Z

NOTE: The following changes require ACE-World database v0.1.8 or newer...
Added NpcLooksLikeObject check for Creature assessment profile fix provided by @OptimShi.
Moved Default do nothing UseDone to WorldObject and removed it from GenericObject.
Added Cow weenie object.
Added Cow, Creature, and Container to the WorldObjectFactory.
Say Hi to the cow in Holtburg.

ogmage78 · 2017-07-31T13:35:40Z

I know we discussed this a while back. Are we going to do one of these per creature family? By that I mean all Tuskers for example will be one object not an object for every different tusker? I think if we do this by weenietype and not weenie - that will allow content creators to add content without having to change server code.

LtRipley36706 · 2017-07-31T15:07:52Z

No need. Cows were a unique WeenieType, all other creatures will simply be Creature.cs

ddevec · 2017-07-31T15:20:36Z

Monster.cs? Monsters have different logical behaviors than creatures (e.g. players don't have scripts they run to attack nearby players).

At least, not on the server side :-P

LtRipley36706 · 2017-07-31T15:46:37Z

There's no distinction between Monsters and Creatures in the client, it would seem from turbine's viewpoint , we all sprang from creature weenietype, at least that i've seen, in the terms of weenie objects used to control actions

….git

Mogwai-TheFurry

In general, you have a LOT of chunks of code you added that are just big comment blocks. Clean them up, please.

Mogwai-TheFurry · 2017-08-02T11:45:06Z

Source/ACE.Database/WorldDatabase.cs

+        {
+            var criteria = new Dictionary<string, object> { { "aceObjectId", aceObjectId } };
+            var objects = ExecuteConstructedGetListStatement<WorldPreparedStatement, AceObjectPropertiesSkill>(WorldPreparedStatement.GetAceObjectPropertiesSkills, criteria);
+            ////objects.ForEach(o =>


no reason to include chunks of commented code in new stuff.

Mogwai-TheFurry · 2017-08-02T11:46:34Z

Source/ACE/Entity/Creature.cs

-            : base(aceObject, guid)
-        {
-        }
+        ////public Creature(AceObject aceObject, ObjectGuid guid)


Deletes are preferred over comments. We have history.

Mogwai-TheFurry · 2017-08-02T11:48:20Z

Source/ACE/Entity/GenericObject.cs

-            : base(guid, aceObject)
-        {
-        }
+        ////public GenericObject(ObjectGuid guid, AceObject aceObject)


At this point the class serves no purpose. It's a commented out class with a constructor. Just remove it.

public static WorldObject CreateWorldObject(AceObject aceO) { WeenieType objWeenieType = (WeenieType?)aceO.WeenieType ?? WeenieType.Generic; switch (objWeenieType) { case WeenieType.LifeStone: return new Lifestone(aceO); case WeenieType.Door: return new Door(aceO); case WeenieType.Portal: return new Portal(aceO); // case WeenieType.PKModifier: // return new PKModifier(aceO); case WeenieType.Cow: return new Cow(aceO); case WeenieType.Creature: return new Creature(aceO); case WeenieType.Container: return new Container(aceO); default: return new GenericObject(aceO); } }

Need a default object. I assume this object will eventually have some worth while code in it.

I'd rather it echo out something to the effect of "Was asked to create a {WeenieType}, but don't know how" or something. GenericObject is useless Object.

except, there are many WeenieType.Generic objects in the database.. its not going to be as useless as you are suggesting

Also, removing it kills every spawn-able object not explicitly handled by CreateWorldObject which does us no good in the now.

ddevec

Just a couple of minor nits.

ddevec · 2017-08-02T18:06:05Z

Source/ACE.Database/WorldDatabase.cs

@@ -39,7 +39,10 @@ private enum WorldPreparedStatement
            GetAceObjectPropertiesPosition = 24,
            GetAceObjectPropertiesSpell = 25,
            GetAceObjectGeneratorLinks = 26,
-            GetMaxId = 27
+            GetMaxId = 27,
+            GetAceObjectPropertiesAttributes,


I don't like the inconsistency here. Either remove the = # from all of them, or put the = # in your enum values.

Og's PR removes them all, I only added new ones from Shard and did not include numbers

ddevec · 2017-08-02T18:08:17Z

Source/ACE/Entity/Cow.cs

+                ////    return;
+                ////}
+
+                if (playerDistanceTo >= radiusSquared)


Not that you have to do it here, but this code appears common to most/all HandleActionOnUse functions. Perhaps we should come up with some other solution than copying it all over the place?

a reasonable suggestion, it probably will be in at least a few more objects as well

….git

ddevec

A few code smells creeping in your last set of commits. Nothing major.

ddevec · 2017-08-03T18:10:21Z

Source/ACE.Entity/AceObject.cs

+            ret.StringProperties.ForEach(c => c.AceObjectId = guid);    
+            ret.GeneratorLinks.ForEach(c => c.AceObjectId = guid);
+
+            // We're not seemingly changing the guids for the Dictionaries below from Clone(), is this bad?


The dictionaries separate the DB layer data and the in-game data, so the guid doesn't need to be adjusted (it will be crated/adjusted on save). This is an okay note, but the comments below it are just clutter.

@ddevec you and @Mogwai-TheFurry need to talk. When we clone a new object from a weenie, we have to do a deep reset of those values. I don't care where it takes place, but it has to happen prior to save. I have this in my code. I made it part of the clone new method. It happens behind the scenes.

In general I dislike the fact that save-time data creeps into our live data-structures, and the smell here is associated with the presence of the redundant data. I believe either implementing the guid update at clone time (as Ripley has done here), or at save time is sufficient. Other than the fact I don't like this data existing at all, I think Ripley's implementation here is fine.

It looks like you and Ripley have essentially created the same code to accomplish this, and it seems to be a fine way to go about doing it to me.

I copied his code in per other discussions just to make it clear what the guid code was doing

ddevec · 2017-08-03T18:12:47Z

Source/ACE/Command/Handlers/AdminCommands.cs

-                session.Network.EnqueueSend(new GameMessagePutObjectInContainer(session, session.Player.Guid, loot, 0),
-                    new GameMessageUpdateInstanceId(loot.Guid, session.Player.Guid, PropertyInstanceId.Container));
-            }
+            WorldObject loot = WorldObjectFactory.CreateNewWorldObject(weenieId);


Just noticed this modifies the player's inventory outside fo an actionchain. This is a race. Please throw a FIXME comment in there for me, since your modifying that code anyway (or wrap this chunk of code in an actionchain).

I assume this code moves to Player (eventually Admin) class anyway

likely most of these commands will need actionchains and relocation.. fwiw

ddevec · 2017-08-03T18:15:17Z

Source/ACE/Entity/Landblock.cs

@@ -104,10 +104,13 @@ public Landblock(LandblockId id)
            var factoryObjects = WorldObjectFactory.CreateWorldObjects(objects);
            factoryObjects.ForEach(fo =>
            {
-                if (!worldObjects.ContainsKey(fo.Guid))
+                if (fo != null)


Under what instance will WorldObjectFactory.CreateWorldObjects populate its object list with a null, and why is that not a bug in WorldObjectFactory.CreateWorldObjects?

technically impossible in the current state of the code. I added the null check when while I was temping in the removal of GenericObject to reconfirm what I already knew. WOF.CreateWorldObjects prevents nulls in results now so the fo null check is just not possible to hit ever, and technically so is the null in CreateWorldObjects as long as we set a default object, which I see no reason no to do.

I think its important to decide if a correct implementation of CreateWorldObjects can or cannot introduce null objects into its list. As it returns a list, I would imagine cannot is the correct behavior.

I am a strong opponent of error-oblivious computing (intentionally ignoring errors), it leads to buggy, inconsistent, and difficult to debug code. Intentionally ignoring values that should be impossible from CreateWorldObjects is a form of error-oblivious computing.

With that said, I would highly recommend removing this check, either replace it with an assertion (Debug.Assert) if your worried about null values, or remove it entirely.

As written, this code indicates CreateWorldObjects can/should return null values in its list, and that's just confusing.

LtRipley36706 added 3 commits July 31, 2017 02:40

Moo

83a55b8

Updating changelog

4d64265

Fix typo

08f0e31

LtRipley36706 added In Progress type:Enhancement type:Feature labels Jul 31, 2017

Fix another typo

c3f6021

Merge branch 'master' into dev-CowTipping

c41462e

LtRipley36706 added 3 commits July 31, 2017 14:01

Pull in attributes from database

ccb80e8

Update changelog

6dbc33b

Merge branch 'dev-CowTipping' of https://github.com/LtRipley36706/ACE…

a7c630e

….git

LtRipley36706 removed the In Progress label Jul 31, 2017

LtRipley36706 added 2 commits August 1, 2017 19:13

Better use of NpcLooksLikeObject

e51aee1

Add CreatureType and refined NpcLooksLikeObject check in Creature

7a1e978

Mogwai-TheFurry suggested changes Aug 2, 2017

View reviewed changes

LtRipley36706 added 2 commits August 2, 2017 12:38

Comment cleanup

b32df5c

Change WorldObjectFactory to work with proper guid propagation

50df1da

ddevec reviewed Aug 2, 2017

View reviewed changes

LtRipley36706 added 7 commits August 2, 2017 14:47

Merge branch 'master' into dev-CowTipping

e4a96c1

More Guid Stuff

fc1857a

Merge branch 'dev-CowTipping' of https://github.com/LtRipley36706/ACE…

1534de2

….git

Minor tweaks

772510c

Refactored UseRadius check and move

2b129a5

Cleanup

b9a8ce7

More cleanup

be4e8b3

ddevec reviewed Aug 3, 2017

View reviewed changes

LtRipley36706 added 2 commits August 3, 2017 15:04

Requested changes

438e0e5

Noting actionchain issue in AdminCommands of which there are many

173e1b6

Mogwai-TheFurry approved these changes Aug 4, 2017

View reviewed changes

LtRipley36706 merged commit 65fbd37 into ACEmulator:master Aug 6, 2017

LtRipley36706 deleted the dev-CowTipping branch February 10, 2018 19:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Cow object #482

Add Cow object #482

LtRipley36706 commented Jul 31, 2017 •

edited

ogmage78 commented Jul 31, 2017

LtRipley36706 commented Jul 31, 2017

ddevec commented Jul 31, 2017 •

edited

LtRipley36706 commented Jul 31, 2017 •

edited

Mogwai-TheFurry left a comment

Mogwai-TheFurry Aug 2, 2017

Mogwai-TheFurry Aug 2, 2017

Mogwai-TheFurry Aug 2, 2017

LtRipley36706 Aug 2, 2017

Mogwai-TheFurry Aug 2, 2017

LtRipley36706 Aug 2, 2017

LtRipley36706 Aug 2, 2017

ddevec left a comment

ddevec Aug 2, 2017

LtRipley36706 Aug 2, 2017

ddevec Aug 2, 2017

LtRipley36706 Aug 2, 2017

ddevec left a comment

ddevec Aug 3, 2017

ogmage78 Aug 3, 2017

ddevec Aug 3, 2017

LtRipley36706 Aug 3, 2017

ddevec Aug 3, 2017

LtRipley36706 Aug 3, 2017

LtRipley36706 Aug 3, 2017

ddevec Aug 3, 2017

LtRipley36706 Aug 3, 2017

ddevec Aug 3, 2017

Add Cow object #482

Add Cow object #482

Conversation

LtRipley36706 commented Jul 31, 2017 • edited

ogmage78 commented Jul 31, 2017

LtRipley36706 commented Jul 31, 2017

ddevec commented Jul 31, 2017 • edited

LtRipley36706 commented Jul 31, 2017 • edited

Mogwai-TheFurry left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ddevec left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ddevec left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LtRipley36706 commented Jul 31, 2017 •

edited

ddevec commented Jul 31, 2017 •

edited

LtRipley36706 commented Jul 31, 2017 •

edited