Skip to content

Update RoomVisual.poly to also accept positions#113

Open
Dessix wants to merge 5 commits into
masterfrom
Dessix-patch-1
Open

Update RoomVisual.poly to also accept positions#113
Dessix wants to merge 5 commits into
masterfrom
Dessix-patch-1

Conversation

@Dessix
Copy link
Copy Markdown
Collaborator

@Dessix Dessix commented Jul 22, 2017

From the documentation:

points | array | An array of points.
Every item should be either an array with 2 numbers (i.e. [10,15]), or a RoomPosition object.

@Dessix Dessix requested review from anisoptera and kotarou July 22, 2017 22:15
Copy link
Copy Markdown
Member

@kotarou kotarou left a comment

Choose a reason for hiding this comment

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

Verified that the PR works.

This misses one example of poly usage, which is also the example given in the screeps documentation - using an array of objects that contain x and y properties (e.g. a path).

Recommend changing to poly(points: Array<{x:number, y:number} | [number, number] | RoomPosition>, style?: PolyStyle): RoomVisual; or equivalent.

Dessix added 2 commits July 22, 2017 19:24
Updated tsconfig.json newLine to meet new Typescript compiler casing requirements
@Dessix
Copy link
Copy Markdown
Collaborator Author

Dessix commented Jul 23, 2017

The changes I've pushed allow objects meeting the { x: number, y: number } structural interface to be used in place of RoomPosition instances. I've tested these as working. This also changes RoomVisual's constructor to the Typescript standard es6 declaration style, wherein the constructor and instance types are separate interfaces, and provides separate subtypes for differentiating the availability of roomName depending on whether or not the "global" constructor was used.

Copy link
Copy Markdown
Member

@kotarou kotarou left a comment

Choose a reason for hiding this comment

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

Visually looks really good to me. Will explicitly test on prod scripts and check soon.

@thaelina
Copy link
Copy Markdown
Contributor

thaelina commented Oct 3, 2017

I tested these changes and was unable to declare a RoomVisual
Error:(39, 19) TS2693:'RoomVisual' only refers to a type, but is being used as a value here.
let vis = new RoomVisual();

Need to add declare const RoomVisual: RoomVisualConstructor;

I know this has been languishing here, but hopefully we can get this repo updated and such

@Dessix
Copy link
Copy Markdown
Collaborator Author

Dessix commented Oct 4, 2017

Added constructor object instance for RoomVisual as per @thaelina's suggestion and rebuilt.

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