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

Units are hostile or friendly #61

Closed
olistic opened this issue May 8, 2018 · 4 comments
Closed

Units are hostile or friendly #61

olistic opened this issue May 8, 2018 · 4 comments
Labels
area: player api Related to the player's API good first issue Looking to contribute? Start here! pkg: abilities Pertaining to @warriorjs/abilities pkg: core Pertaining to @warriorjs/core pkg: units Pertaining to @warriorjs/units status: has pr Issues with an accompanying Pull Request type: enhancement A potential improvement to something that already exists

Comments

@olistic
Copy link
Owner

olistic commented May 8, 2018

Units are "hostile" or "friendly", instead of "enemy" or "captive". This gives greater flexibility.

With this change:

Docs and official levels tips like this one will need update as well.

UPDATE: Let's also pass a bound parameter that defaults to false to the Unit's constructor, and set it to true in the Captive unit definition.

@olistic olistic added good first issue Looking to contribute? Start here! pkg: core Pertaining to @warriorjs/core pkg: units Pertaining to @warriorjs/units area: player api Related to the player's API type: enhancement A potential improvement to something that already exists labels May 8, 2018
@olistic olistic added the pkg: abilities Pertaining to @warriorjs/abilities label May 8, 2018
@froehlichA
Copy link

froehlichA commented May 14, 2018

If you plan to support additional characters that are friendly, but not captive (for example, NPC shopkeepers) further down the road, I would suggest adding the .isFriendly() next to the .isCaptive() method, so that you have two super types: "hostile" and "friendly", and "captive" as a sub type of "friendly".

@olistic
Copy link
Owner Author

olistic commented May 14, 2018

Hello @froehlichA! Thanks for your interest in the project. Regarding this issue, my plan is to make the core as generic as possible. For that matter, being able to distinguish between hostile and friendly units is enough, leaving the semantics of what a captive is to the towers (some towers may not even define them). For the existing towers, for example, a captive is a unit that is friendly and is bound. With that in mind, do you think there's a benefit for preserving .isCaptive()?

@froehlichA
Copy link

froehlichA commented May 14, 2018

Currenty, the beginner tower uses the units imported from warriorjs/units. Why are these units defined in this seperate package, and not in the beginner tower source? Are they there for other tower creators to expand upon?

Personal Opinion: If you want to make the core as generic as possible, I would just put basic unit types that provide a basis to build upon in the warriorjs/units package and leave the rest (like Archer, Sludge, or Wizard) to the tower creators.

Concerning the .isCaptive() method, my personal opinion is that the .isCaptive() should be removed from the Space API and replaced with the .isFriendly() method, like you mentioned, but that tower creators should then be able to define the appropiate methods on the units themselves instead.

An Example:

//warrior-tower-community/units/Shopkeeper.js
const Shopkeeper = {
  name: 'Shopkeeper',
  character: '$',
  maxHealth: 7,
  friendly: true,
  detectionFunction: 'isShopkeeper'
}
export default Shopkeeper;

Warriorjs should then automatically expose the method unit.isShopkeeper() (or something similar) in their API.

With this principle, the tower creators themselves would be able to give players the possibility to check if an unit is a specific unit type or not, because players may want to take different actions if a unit is a friendly shopkeeper, a friendly captive or another friendly unit.

TLDR: Generally, I agree with you that there should only be detection of alignment (.isHostile() and .isFriendly()) in the core, but also leave definition of detection of unit types (.isCaptive(), .isArcher(), ...) to the tower creators, and only expose those through the API.

@olistic
Copy link
Owner Author

olistic commented May 14, 2018

Currenty, the beginner tower uses the units imported from warriorjs/units. Why are these units defined in this seperate package, and not in the beginner tower source? Are they there for other tower creators to expand upon?

Currently, the two official warriorjs towers are beginner and intermediate. As both shared units, abilities and effects, I decided to create separate packages for them. That allowed me to reuse code in both towers, and also allows other tower makers to use them if they want.

Personal Opinion: If you want to make the core as generic as possible, I would just put basic unit types that provide a basis to build upon in the warriorjs/units package and leave the rest (like Archer, Sludge, or Wizard) to the tower creators.

What I mean by "the core" is the @warriorjs/core package, not all the packages inside this repo. With that definition, the core is already agnostic to unit types, abilities and effects. Those are left entirely to tower makers.

(...) tower creators should then be able to define the appropiate methods on the units themselves instead.

I like this idea, I think it's worth exploring. Would you mind submitting a new issue to discuss this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: player api Related to the player's API good first issue Looking to contribute? Start here! pkg: abilities Pertaining to @warriorjs/abilities pkg: core Pertaining to @warriorjs/core pkg: units Pertaining to @warriorjs/units status: has pr Issues with an accompanying Pull Request type: enhancement A potential improvement to something that already exists
Projects
None yet
Development

No branches or pull requests

2 participants