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

Namespace and type naming collisions #26

Closed
MarioE opened this issue Jul 27, 2016 · 6 comments
Closed

Namespace and type naming collisions #26

MarioE opened this issue Jul 27, 2016 · 6 comments

Comments

@MarioE
Copy link
Contributor

MarioE commented Jul 27, 2016

There are a few instances of shared namespace and type names:

  • class Orion and namespace Orion
  • class Item and namespaces Orion.Entities.Item, Orion.Events.Item
  • class Npc and namespaces Orion.Entities.Npc, Orion.Events.Npc
  • class Player and namespaces Orion.Entities.Player, Orion.Events.Player
  • class Projectile and namespaces Orion.Entities.Projectile, Orion.Events.Projectile

This could lead to a few annoyances later on, and should probably be fixed now. One solution would be to pluralize Orion.Entities.<EntityName> (and place the events in there too, or pluralize that as well). I'm not sure how we should deal with Orion.Orion, though.

@Ijwu
Copy link
Contributor

Ijwu commented Jul 27, 2016

I think renaming the Orion namespace to something like OrionApi is fitting and acceptable.

What about prefixing the entity names? Something like OrionItem, OrionNpc, etc. That way we have differentiation in the code as from the Terraria entities. (As well as name differentiation).

We can probably then leave the namespaces the same.

@MarioE
Copy link
Contributor Author

MarioE commented Jul 27, 2016

I do like prefixing the entity names, but you would still have a conflict with the namespace and the Terraria entity. Since the concrete types are going into TShock, we would name them TshockItem, TshockNpc, etc.

Also, something to note is that our type names conflicting with Terraria names shouldn't make too much of a difference in our name decisions, since our goal is to eventually have consumers not need to reference anything in the Terraria namespace.

@Ijwu
Copy link
Contributor

Ijwu commented Jul 27, 2016

Prefixing them with TShock (or Tshock, whatever) is good. I would agree with pluralizing the namespaces.

Note: Ignore accidental comment+close

@Ijwu Ijwu closed this as completed Jul 27, 2016
@Ijwu Ijwu reopened this Jul 27, 2016
@MarioE
Copy link
Contributor Author

MarioE commented Jul 28, 2016

If there are no objections to the namespace renaming, then I will do this tonight.

@Ijwu
Copy link
Contributor

Ijwu commented Jul 28, 2016

Please detail the exact changes you'll make, for the sake of having it on record+everyone being on the same page.

@MarioE
Copy link
Contributor Author

MarioE commented Jul 28, 2016

  • Orion.* -> OrionApi.*
  • Orion.Entities.<name> -> OrionApi.Entities.<name>s
  • Orion.Events.<name> -> OrionApi.Events.<name>s

EDIT: where <name> != World

@MarioE MarioE closed this as completed in e63731f Jul 29, 2016
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

No branches or pull requests

2 participants