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

Reformed inventory operations, and fixed bugs #253

Merged
merged 1 commit into from
May 10, 2022

Conversation

sakipvp
Copy link
Contributor

@sakipvp sakipvp commented May 9, 2022

No description provided.

/// Creates a clone from this.
/// </summary>
/// <returns>The clone <see cref="InventoryItem"/> instance.</returns>
public InventoryItem CreateClone()
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you consider implementing IClonable? Might be useful for other Classes, too. https://dotnetcoretutorials.com/2020/09/09/cloning-objects-in-c-and-net-core/ I like the idea of a CloningService which is called in the IClonable.Clone() method.

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 thought about it. I prefer the ICloneable interface with the T Clone () method.

case 0:
ParseInventoryToInventory(packet);
//e.g when equipping a Bow (see ammo)
case 0: // InventoryToInventory
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you create an enum for the type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Like this? enum InventoryOperation{ InventoryToInventory = 0, StorageToStorage = 1

Copy link
Owner

Choose a reason for hiding this comment

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

I will do it.

@sakipvp
Copy link
Contributor Author

sakipvp commented May 9, 2022

I just see now, that the old methods were static. So make the new methods static too?

@SDClowen
Copy link
Owner

SDClowen commented May 10, 2022

I just see now, that the old methods were static. So make the new methods static too?

I merged your pull request. I am working on inventory system.

@SDClowen SDClowen merged commit a56b5dd into SDClowen:master May 10, 2022
@sakipvp
Copy link
Contributor Author

sakipvp commented May 10, 2022

You know, my master :-)
Good brainstorming!

@sakipvp sakipvp deleted the InventoryOperationResponse branch May 12, 2022 05:38
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.

3 participants