-
Notifications
You must be signed in to change notification settings - Fork 9
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
Basic Inventory Management #41
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for your interest and PR!
I've left some comments on a few things I think should be cleaned up before merging.
Let me know if you have any questions or concerns regarding them.
public void SetEnemy() | ||
{ | ||
HostilityInfo = new HostilityInfoData() | ||
{ | ||
Flags = HostilityInfoData.HostilityFlags.Faction, FactionId = 1 | ||
}; | ||
Character_ObserverView.HostilityInfoProp = HostilityInfo; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really needed if the faction id is hardcoded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method was added as a mistake and is now removed. Sorry!
@@ -475,7 +478,7 @@ public void ApplyLoadout(CharacterLoadout loadout) | |||
var chassis = new SlottedItem | |||
{ | |||
SdbId = loadout.ChassisID, | |||
SlotIndex = 0, | |||
SlotIndex = (byte) LoadoutSlotType.GearTorso, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This value should be 255 to match with captures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
@@ -484,7 +487,7 @@ public void ApplyLoadout(CharacterLoadout loadout) | |||
var backpack = new SlottedItem | |||
{ | |||
SdbId = loadout.BackpackID, | |||
SlotIndex = 0, | |||
SlotIndex = (byte) LoadoutSlotType.GearReactor, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This value should be 255 to match with captures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
@@ -495,7 +498,7 @@ public void ApplyLoadout(CharacterLoadout loadout) | |||
Item = new SlottedItem | |||
{ | |||
SdbId = loadout.SlottedItems.GetValueOrDefault(LoadoutSlotType.Primary), | |||
SlotIndex = 0, | |||
SlotIndex = (byte) LoadoutSlotType.Primary, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This value should be 255 to match with captures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
@@ -520,7 +523,7 @@ public void ApplyLoadout(CharacterLoadout loadout) | |||
Item = new SlottedItem | |||
{ | |||
SdbId = loadout.SlottedItems.GetValueOrDefault(LoadoutSlotType.Secondary), | |||
SlotIndex = 0, | |||
SlotIndex = (byte) LoadoutSlotType.Secondary, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This value should be 255 to match with captures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
@@ -937,7 +957,25 @@ public static void GenerateLoadoutAndItems(CharacterInventory inventory, Loadout | |||
} | |||
|
|||
pvpConfig.Items = pvpItems.ToArray(); | |||
|
|||
pvpConfig.Visuals = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Certainly not needed for pvp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
pveConfig.Items = pveItems.ToArray(); | ||
|
||
pveConfig.Visuals = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you checked if this has any side effects, I don't think we should send anything here if we don't have it equipped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed here. This will now be added and removed as needed by CharacterInventory.EquipItemByGUID
switch (slot) | ||
{ | ||
case LoadoutSlotType.Glider: | ||
_character.StaticInfo = _character.StaticInfo with { LoadoutGlider = _items[guid].SdbId }; | ||
break; | ||
case LoadoutSlotType.Vehicle: | ||
_character.StaticInfo = _character.StaticInfo with { LoadoutVehicle = _items[guid].SdbId }; | ||
break; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should instead just call SetStaticInfo so that the ObserverView and BaseController are updated too.
Could consider doing so in ApplyLoadout instead as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in this instance
// _items.Remove(guid); | ||
// var itemToStore = CreateItem(currentItem); | ||
|
||
SendFullInventory(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should just implement a function like SendItemUpdate/SendResourceUpdate to update just the loadout and related item instead, since this is pretty wasteful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented SendEquipmentChanges to replace this, which sends the old and new item (or just one of them, as necessary), as well as the loadouts.
|
||
Player.Inventory.EquipVisualBySdbId(loadoutId, visualSlot, slot, sdb_id); | ||
Player.CharacterEntity.CurrentLoadout.GliderID = sdb_id; | ||
Player.NetChannels[ChannelType.ReliableGss].SendIAero(Character_ObserverView, this.EntityId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are only sending to the local player here so other players won't get this keyframe. But you should not need to force send a keyframe here after correcting the static info calls above. So this line should just be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
Hi @Xsear, thanks for looking into my PR. I've fixed all of the comments, and did some restructuring of EquipItemByGUID to make it more readable and avoid some edge cases. It seems to be a bit better in game now as well that I'm not sending the whole inventory everytime. |
* Basic Inventory Management * Equip and use Glider/Vehicle hotkeys * Remove redundant code, minor changes. * Refactor Equip methods * Send Partial Inventory * Minor cleanup
Handles the following Messages:
Does not include Module installation.