Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inventory - Ready for Merge #484

Merged
merged 28 commits into from
Aug 14, 2017
Merged

Conversation

ogmage78
Copy link
Contributor

@ogmage78 ogmage78 commented Aug 2, 2017

2017-08-01

[Og II]

Refactored again on Mogwai's feedback

This is getting so large, I need to stop here - this is a good spot - adds significant functionality but with a few known bugs.

  • Inventory will save with the container and load again in the correct positions.
  • Character Description Event is now wired up for inventory.
  • removed containers copy of aceObject Inventory - just make the list accessible.
  • Updated Clone so it you pass a new guid, it resets all the associated child tables.
  • implemented placement IntProperty 65 used to track slot in Container
  • Created supporting methods to manage pack order
  • Enhanced the debug log message for DBDEBUG
  • Small temp fix to stutter when attempting to shift walk or shift jump. We still need to really understand autonomous
  • Created new view vw_ace_inventory_object to expose ability to pull by container.
  • general clean up comments and whitespace
  • Inventory loading - working
  • TODO.
  • 1 Add in capacity checks
  • 2 bug still in picking up items off the ground and saving to database.
  • 3 bug with reloaded items saved in a secondary side pack.

Copy link
Member

@LtRipley36706 LtRipley36706 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possible path worth exploring, it might need to pull in some of the changes from pr #482 to fully work like you might expect

@@ -1015,17 +1015,23 @@ public static void HandleCI(Session session, params string[] parameters)
WorldObject testContainer = LootGenerationFactory.CreateTestWorldObject(session.Player, weenieId);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

            WorldObject newObj = WorldObjectFactory.CreateNewWorldObject(weenieId);
            
                newObj.ContainerId = session.Player.Guid.Full;
                newObj.Placement = 0;
                session.Player.AddToInventory(newObj);
                session.Player.TrackObject(newObj);
                session.Network.EnqueueSend(new GameMessagePutObjectInContainer(session, session.Player.Guid, newObj, 0),
                    new GameMessageUpdateInstanceId(newObj.Guid, session.Player.Guid, PropertyInstanceId.Container));
            }
        }

try this instead

@@ -825,8 +826,10 @@ public static void HandleWeapons(Session session, params string[] parameters)
{
foreach (uint weenieId in weaponsTest)
{
WorldObject loot = LootGenerationFactory.CreateTestWorldObject(session.Player, weenieId);
AceObject lootAceObject = (AceObject)DatabaseManager.World.GetAceObjectByWeenie(weenieId).Clone(GuidManager.NewItemGuid().Full);
WorldObject loot = new GenericObject(lootAceObject);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WorldObject loot = WorldObjectFactory.CreateNewWorldObject(weenieId);

instead of AceObject + WorldObject stuff

Copy link
Contributor Author

@ogmage78 ogmage78 Aug 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that first - but it dies not look like it really works. If you try to persist it, you will see that what you are doing behind the scenes, is making a world object which is made from an aceObject. Then you do a clone and assign a new guid. It looks like a new object and it is, but the problem is that all of the aceObject contained in your new world object have the aceobjectid of the item you cloned from. It does not matter as long as you are just spawning something that will never be persisted. If you actually try to save that to the database, it blows up. This actually works as coded.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the solution here is to make CreateNewWorldObject work. CreateNewWorldObject is meant to be the primary tool for generating loot. It currently only takes a weenie, but should probably be modified to accept a Loot Tier (int) as well.

This functionality should be an easy one liner, not a complicated daisy-chain of methods like Og had to implement.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I will work on making the other method work and refactor my code to use that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See this change in 482

Think it does what you're expecting, assuming your clone(guid) changes are also merged

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does part of it. The DB layer should be assigning the objectGuid of the child objects, but it doesn't hurt (and is probably better) to do a deep reassignment of the guid.

@@ -849,11 +852,13 @@ public static void HandleSplits(Session session, params string[] parameters)
{
foreach (uint weenieId in splitsTest)
{
WorldObject loot = LootGenerationFactory.CreateTestWorldObject(session.Player, weenieId);
AceObject lootAceObject = (AceObject)DatabaseManager.World.GetAceObjectByWeenie(weenieId).Clone(GuidManager.NewItemGuid().Full);
WorldObject loot = new GenericObject(lootAceObject);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WorldObject loot = WorldObjectFactory.CreateNewWorldObject(weenieId);

instead of AceObject + WorldObject stuff

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above.

…re a code base between world and shard so in this instance must stay in sync. More inventory loading work.
@@ -1798,6 +1808,20 @@ public object Clone(uint guid)
{
AceObject ret = (AceObject)Clone();
ret.AceObjectId = guid;
// We are cloning a new AceObject with a new AceObjectID - need to set this to false. Og II
ret.HasEverBeenSavedToDatabase = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also make it Dirty. ret.IsDirty = true;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do

@@ -825,8 +826,10 @@ public static void HandleWeapons(Session session, params string[] parameters)
{
foreach (uint weenieId in weaponsTest)
{
WorldObject loot = LootGenerationFactory.CreateTestWorldObject(session.Player, weenieId);
AceObject lootAceObject = (AceObject)DatabaseManager.World.GetAceObjectByWeenie(weenieId).Clone(GuidManager.NewItemGuid().Full);
WorldObject loot = new GenericObject(lootAceObject);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the solution here is to make CreateNewWorldObject work. CreateNewWorldObject is meant to be the primary tool for generating loot. It currently only takes a weenie, but should probably be modified to accept a Loot Tier (int) as well.

This functionality should be an easy one liner, not a complicated daisy-chain of methods like Og had to implement.

var invList = inventory.OrderBy(o => o.Value.Placement);
uint place = 0;
AceObject saveableCopy;
foreach (var keyValuePair in invList)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This loop doesn't make sense to me. Can you explain what it does?

Copy link
Contributor Author

@ogmage78 ogmage78 Aug 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you either put something in your backpack or you just move it around to a new slot - everything in the pack with a placement >= the placement slot you are going in has to move. Put 4 things in your pack. They are in slots 0, 1, 2, 3 Now drag item in slot 3 to slot 0. 3 is now is set to 0, 0 is set to 1 and 1 is set to 2 and 2 is set to 3. Not every move ends up with a total inventory shift. Only drag to the first of the list does that. If you drag to the middle, only everything past that has to shift. If you drag to the last open slot, nothing moves. By default, when you pick something up off the ground if you do not drag it to a slot, it goes to position 0. There may be a better way to do that. This works correctly in all cases with the minimum number of moves. (updates). More than happy to refactor if there is a better way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a mini towers of hanoi ;) Just kidding that is what it made me think of laying out what was happening on a piece of paper.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured that's what it was, but it's coded rather oddly. I don't think we need to be saving on every ordering alteration. We can let the periodic save handle this sort of stuff.

There are easier ways to do this:
invList.Where(i => i.Placement >= newItemPlacement).ForEach(i => i.Placement++);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, need to check for container limitations? Ie, holds 24, already at capacity, etc. Method should probably return a boolean indicator of success so the caller can act accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I knew there had to be a better way. Thanks Mogwai! I will also add in capacity checks as part of this work.

@@ -51,14 +52,54 @@ public Container(AceObject aceObject, ObjectGuid guid)
{
}

public virtual void ContainerPlacement(WorldObject inventoryItem)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a really big fan of this function name. What does ContainerPlacement mean? I would assume it returns the container placement of the item, but that's clearly not it, it removes an inventory item, then does some other stuff. Perhaps RemoveItemFromInventory? Or would that be improper for some reason?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like: PlaceItemInContainer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do

saveableCopy = keyValuePair.Value.SnapShotOfAceObject();
saveableCopy.ClearDirtyFlags();
saveableCopy.Placement = place;
DatabaseManager.Shard.SaveObject(saveableCopy, null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still not 100% sure what this funciton does. But shouldn't saving these objects be piggy-backed on saving the player, not done on every inventory reshuffle?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going to put this save in a #if directive it makes it easy to debug if it persists right away. I will hook this in with character save which is how it will work.

saveableCopy = inventoryItem.SnapShotOfAceObject();
saveableCopy.ClearDirtyFlags();
saveableCopy.Placement = inventoryItem.Placement;
DatabaseManager.Shard.SaveObject(saveableCopy, null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above.

… the world from spawning - have to find that bug.
…ved changes to vw_ace_object (can use that has to have position) created very simple view for inventory loading. Wired up inventory section of inventory and stubbed in wielded.
@ogmage78 ogmage78 changed the title Doctor Strangelove or: How I Learned to Stop Worrying and Love Multithreading REVIEW ONLY Doctor Strangelove or: How I Learned to Stop Worrying and Love Multithreading NEW REVIEW REQUESTED Aug 4, 2017
Copy link
Contributor

@Mogwai-TheFurry Mogwai-TheFurry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that this is shaping up and I can read it more easily, I'm not a fan that we maintain a duplicate list of inventory in Container (with the original/primary being AceObject). IMO, Container should act on the AceObject.Inventory collection, which removes a lot of the issues you have with clearing and duplicating lists.

@LtRipley36706
Copy link
Member

@Mogwai-TheFurry basically a function call, Container.AddToInventory(worldobject) would add it to the collection of inventory, recalc the burden of the container and send all the appropriate createobject and privateupdateints with the RemoveFromInventory doing the removeobject msg instead yeah?

Copy link
Member

@LtRipley36706 LtRipley36706 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some object stuff I noticed which could pose issues later on

session.Network.EnqueueSend(new GameMessageCreateObject(invItem));
foreach (var invItem in Inventory.Values)
{
var inv = new GenericObject(invItem);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should switch to var inv = CreateWorldObject(invItem);


var containers = inventory.Where(wo => wo.Value.ItemCapacity > 0).ToList();
return containers.Any(cnt => ((Container)cnt.Value).inventory.ContainsKey(inventoryItemGuid));
var containers = Inventory.Where(wo => wo.Value.ItemsCapacity > 0).ToList();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of testing containers by ItemsCapacity, test by WeenieType or even object type (Container) if the objects in Inventory are WorldObjects?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was legacy from when we did not have types set and everything was a debug object, I will fix. This was a lot more involved than I thought, so I just overlooked that. Thanks for catching it!

}

public virtual WorldObject GetInventoryItem(ObjectGuid objectGuid)
{
var containers = inventory.Where(wo => wo.Value.ItemCapacity > 0).ToList();
if (Inventory.ContainsKey(objectGuid))
return new GenericObject(Inventory[objectGuid]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not all objects are GenericObject, use CreateWorldObject to get proper object

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I will fix

if (Inventory.ContainsKey(objectGuid))
return new GenericObject(Inventory[objectGuid]);

var containers = Inventory.Where(wo => wo.Value.ItemsCapacity > 0).ToList();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wo.GetType == Container or wo.WeenieType == WeenieType.Container here maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was legacy from when we did not have types set and everything was a debug object, I will fix. This was a lot more involved than I thought, so I just overlooked that. Thanks for catching it!

@ogmage78
Copy link
Contributor Author

ogmage78 commented Aug 5, 2017

@LtRipley36706 Made those changes - updating now.

…g saving items in secondary packs to the db.
@ogmage78 ogmage78 changed the title Doctor Strangelove or: How I Learned to Stop Worrying and Love Multithreading NEW REVIEW REQUESTED Doctor Strangelove or: How I Learned to Stop Worrying and Love Multi-threading READY FOR FINAL REVIEW & MERGE Aug 6, 2017
CreatureType = 2,
PaletteTemplate = 3,
ClothingPriority = 4,
EncumbranceVal = 5, // ENCUMB_VAL_INT,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lost note in formatting, please restore so we can track what we changed from client/aclogview

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We align the formatting on all of the other files - you want it unaligned? I am not sure I follow?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no issues with the alignment, it was the note // ENCUMB_VAL_INT that was lost.. ACE calls it EncumbranceVal, client and aclogview call it ENCUMB_VAL_INT

ItemsCapacity = 6,
ContainersCapacity = 7,
Mass = 8,
ValidLocations = 9, // LOCATIONS_INT,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lost note in formatting, please restore so we can track what we changed from client/aclogview

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We align the formatting on all of the other files - you want it unaligned? I am not sure I follow?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no issues with the alignment, it was the note // LOCATIONS_INT that was lost.. ACE calls it ValidLocations, client and aclogview call it LOCATIONS_INT

@@ -75,6 +75,7 @@ public enum WeenieType : uint
SocialManager,
Pet,
PetDevice,
CombatPet
CombatPet,
Foci
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Foci did not have their own WeenieType. They were assigned to WeenieType.Generic

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know, I needed to add one

@@ -1460,7 +1460,8 @@ protected WorldObject(AceObject aceObject)
: this(new ObjectGuid(aceObject.AceObjectId))
{
AceObject = aceObject;

SetWeenieHeaderFlag();
SetWeenieHeaderFlag2();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these shouldn't be needed, the WeenieHeaderFlags are set when an object reads the field. If its not updating, its a bug for sure

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was needed it was not working.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe worth a todo/fixme.. we shouldn't have to think too much about these

Writer.Write(inv.Value.AceObjectId);
if ((WeenieType)inv.Value.WeenieType == WeenieType.Container)
Writer.Write((uint)ContainerType.Container);
if ((WeenieType)inv.Value.WeenieType == WeenieType.Foci)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

technically foci weren't containers, as they were classified as WeenieType.Generic.. They do have RequireBackPackSlot flag on their ObjDesc however

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are passed as containers in the character description event.

case WeenieType.Container:
Writer.Write((uint)ContainerType.Container);
break;
case WeenieType.Foci:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above comments, possible issue with categorization

@ACEmulator ACEmulator deleted a comment from ogmage78 Aug 6, 2017
@ogmage78 ogmage78 changed the title Doctor Strangelove or: How I Learned to Stop Worrying and Love Multi-threading READY FOR FINAL REVIEW & MERGE Doctor Strangelove or: How I Learned to Stop Worrying and Love Multi-threading MERGE BROKE THIS - NOT READY FOR MERGE Aug 6, 2017
@ogmage78 ogmage78 changed the title Doctor Strangelove or: How I Learned to Stop Worrying and Love Multi-threading MERGE BROKE THIS - NOT READY FOR MERGE Doctor Strangelove or: How I Learned to Stop Worrying and Love Multi-threading READY FOR FINAL REVIEW & MERGE Aug 7, 2017
@@ -1220,6 +1220,12 @@ public string DisplayName
set { SetBoolProperty(PropertyBool.RequiresBackpackSlot, value); }
}

public bool UseBackpackSlot
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels like this shouldn't be in AceObject and instead in WorldObject. I follow why you put it here though. GameEventPlayerDescription reads things as AceObject and not WorldObject.

Maybe slap a few fixme/todos to correct the anomalous property/object usage so when someone gets around to bringing GameEventPlayerDescription in line with expectations this oddity gets corrected?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It really has to be in both places because to do what Mogwai wanted, I don't make inventory a world object until it is used or wielded. It is only ready only and makes the code much simpler to follow.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is necessary.

Copy link
Member

@LtRipley36706 LtRipley36706 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

notes added, only one real question regarding saving to db flag

ret.GeneratorLinks.ForEach(c => c.AceObjectId = guid);

// No need to change Dictionary guids per DDEVEC
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bad merge? some comment got axed, some things got duplicated

@@ -1817,8 +1841,7 @@ public object Clone(uint guid)
AceObject ret = (AceObject)Clone();
ret.AceObjectId = guid;
// We are cloning a new AceObject with a new AceObjectID - need to set this to false. Og II
ret.HasEverBeenSavedToDatabase = false;
ret.IsDirty = true;
ret.SetDirtyFlags();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HasEverBeenSavedToDatabase possible issue in removing this..

is it possible for something to be HasEverBeenSavedToDatabase == true and IsDirty == true? I'd assume something could be dirty that already has been saved to the database, so I don't follow the change entirely.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is not enough to set the flag on the top level. This looked like it was working because until this work, we never really saved items to the database. What I put in - I tested and it works - you have to do a deep set the setDirtyFlags that primes it to save to the database for the first time from a clone. The flags are maintained automatically after that first save as items are changed. It is a good system Mogwai put in, we just needed to debug the first real saves. This does that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bingo.

public void ClearDirtyFlags()
{
this.IsDirty = false;
this.HasEverBeenSavedToDatabase = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above comment re HasEverBeenSavedToDatabase

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is correct. This call is only to be used on first save after clone.

public void SetDirtyFlags()
{
this.IsDirty = true;
this.HasEverBeenSavedToDatabase = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above comment re HasEverBeenSavedToDatabase

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is correct. This call is only to be used on first save after clone.

public void SetDirtyFlags()
{
this.IsDirty = true;
this.HasEverBeenSavedToDatabase = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above comment re HasEverBeenSavedToDatabase

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is correct. This call is only to be used on first save after clone.

@@ -1371,14 +1376,17 @@ public bool GetVirtualOnlineStatus()

private void SendSelf()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just something I noticed in retail pcaps, but haven't investigated much...

following the CreatePlayer and Player Description Event there is often a DeleteObject which deletes yourself followed by a CreateObject which creates yourself. The DeleteObject message isn't fully handled by aclogview so there could some something else in that packet which puts this in to better context

Not sure I get why, but worthing noting and investigating.

@@ -1460,7 +1460,8 @@ protected WorldObject(AceObject aceObject)
: this(new ObjectGuid(aceObject.AceObjectId))
{
AceObject = aceObject;

SetWeenieHeaderFlag();
SetWeenieHeaderFlag2();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe worth a todo/fixme.. we shouldn't have to think too much about these

WorldObject wo = CreateTestWorldObject(player, weenieList[i].WeenieClassId);
WorldObject wo = WorldObjectFactory.CreateNewWorldObject(weenieList[i].WeenieClassId);
if (wo.WeenieType == WeenieType.Container)
player.Session.Network.EnqueueSend(new GameEventViewContents(player.Session, wo.SnapShotOfAceObject()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any time a container is created in the world if you are in range you are shown the contents before opening?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is only send to the player that has the container

public class GameEventViewContents : GameEventMessage
{
public GameEventViewContents(Session session, uint objectId)
public GameEventViewContents(Session session, AceObject container)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feels like this should be a WorldObject.. AceObject is behind the scenes usually

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, mogway made me change inventory to only make world objects when needed.

changelog.md Outdated
@@ -1,5 +1,25 @@
# ACEmulator Change Log


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bad merge in here

@ogmage78 ogmage78 closed this Aug 8, 2017
@ogmage78 ogmage78 changed the title Doctor Strangelove or: How I Learned to Stop Worrying and Love Multi-threading READY FOR FINAL REVIEW & MERGE Inventory - Ready for Merge Aug 12, 2017
@ogmage78
Copy link
Contributor Author

ogmage78 commented Aug 12, 2017

As I put in the change log, there are some known bugs, but I think this is a good start. I am sure there are some better ways to program this. I would love to learn them, but at this time, I will be taking a step back from the project. I thought this added value so I am reopening this. If you want to merge it so someone else can pick this up - great. If not, then just close it without merge. I had hoped I would be able to keep working on the project, but that does not seem likely at this time. A big thank you to @Mogwai-TheFurry for taking the time to help a very out of date programmer. I wish you guys the very best.

@ogmage78 ogmage78 reopened this Aug 12, 2017
Copy link
Contributor

@Mogwai-TheFurry Mogwai-TheFurry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very minor stuff. Looks great, Og!

@@ -1220,6 +1220,12 @@ public string DisplayName
set { SetBoolProperty(PropertyBool.RequiresBackpackSlot, value); }
}

public bool UseBackpackSlot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is necessary.

@@ -1817,8 +1841,7 @@ public object Clone(uint guid)
AceObject ret = (AceObject)Clone();
ret.AceObjectId = guid;
// We are cloning a new AceObject with a new AceObjectID - need to set this to false. Og II
ret.HasEverBeenSavedToDatabase = false;
ret.IsDirty = true;
ret.SetDirtyFlags();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bingo.

@@ -792,14 +794,14 @@ public void ScheduleItemTransferInContainer(ActionChain chain, ObjectGuid wo, Co
}
}

public void ScheduleItemTransfer(ActionChain chain, ObjectGuid wo, ObjectGuid container)
public void ScheduleItemTransfer(ActionChain chain, ObjectGuid wo, ObjectGuid container, uint placement = 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the terminology we've been using as a semi-standard is "QueueItemTransfer". No biggie.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed.

}

private void Log(string message)
{
log.Debug($"LB {id.Landblock.ToString("X")}: {message}");
log.Debug($"LB {id.Landblock:X}: {message}");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ToString was right - it would have printed the landblock Id in Hex. You're now only logging the "X" component.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed - damn resharper lead me astray .... ;)

changelog.md Outdated
@@ -18,6 +39,7 @@
### 2017-08-06
[Ripley]
* Added Scroll weenie object.
>>>>>>> 7d22ad27bd89e4462abb1f358c37f0891ea3d9a3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge artifact

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@Mogwai-TheFurry Mogwai-TheFurry merged commit d29bc3d into ACEmulator:master Aug 14, 2017
@ogmage78 ogmage78 deleted the inventory branch August 15, 2017 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants