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.

Sign up for GitHub

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

feat: add RoomTerrain.getRawBuffer() #247

Merged
merged 4 commits into from
Dec 2, 2023
Merged

feat: add RoomTerrain.getRawBuffer() #247

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3319602
feat: add RoomTerrain.getRawBuffer()
admon84 Sep 20, 2023
002731c
fix: bump version
admon84 Sep 20, 2023
f36171a
fix: incorporate changes recommended by @DiamondMofend and update tests
admon84 Sep 23, 2023
7d553fa
fix: replace TypedArray helper with inline type
admon84 Sep 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion dist/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Type definitions for Screeps 3.3.3
// Type definitions for Screeps 3.3.4
// Project: https://github.com/screeps/screeps
// Definitions by: Nhan Ho <https://github.com/NhanHo>
// Bryan <https://github.com/bryanbecker>
Expand Down Expand Up @@ -4044,6 +4044,12 @@ interface RoomTerrain {
* @return number Number of terrain mask like: TERRAIN_MASK_SWAMP | TERRAIN_MASK_WALL
*/
get(x: number, y: number): 0 | TERRAIN_MASK_WALL | TERRAIN_MASK_SWAMP;
/**
* Get copy of underlying static terrain buffer.
* @param destinationArray (optional) A typed array view in which terrain will be copied to.
* @return Uint8Array Copy of underlying room terrain as a new Uint8Array typed array of size 2500.
*/
getRawBuffer(destinationArray?: Uint8Array): Uint8Array;
}

interface RoomTerrainConstructor extends _Constructor<RoomTerrain> {
Expand Down
17 changes: 17 additions & 0 deletions dist/screeps-tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1126,6 +1126,23 @@ function resources(o: GenericStore): ResourceConstant[] {
}

const enemyTerrain = new Room.Terrain("W2N5");

const raw = myTerrain.getRawBuffer();

for (let y = 0; y < 50; y++) {
for (let x = 0; x < 50; x++) {
const code = raw[y * 50 + x];
if (code === 0) {
/*plain*/
}
if (code & TERRAIN_MASK_SWAMP) {
/*swamp*/
}
if (code & TERRAIN_MASK_WALL) {
/*wall*/
}
}
}
}

// Creep.body
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "typed-screeps",
"version": "3.3.3",
"version": "3.3.4",
Copy link
Member

Choose a reason for hiding this comment

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

does not need to bump version here

"description": "Strong TypeScript declarations for the game Screeps.",
"repository": "screepers/typed-screeps",
"types": "./dist/index.d.ts",
Expand Down
6 changes: 6 additions & 0 deletions src/room-terrain.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ interface RoomTerrain {
* @return number Number of terrain mask like: TERRAIN_MASK_SWAMP | TERRAIN_MASK_WALL
*/
get(x: number, y: number): 0 | TERRAIN_MASK_WALL | TERRAIN_MASK_SWAMP;
/**
* Get copy of underlying static terrain buffer.
* @param destinationArray (optional) A typed array view in which terrain will be copied to.
* @return Uint8Array Copy of underlying room terrain as a new Uint8Array typed array of size 2500.
Copy link
Member

@DiamondMofeng DiamondMofeng Sep 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Although the documentation says that this method may return ERR_INVALID_ARGS, I do not see it in the source of screeps engine. So it's okay to ignore it. Anyway, maybe we should leave a comment about it here.

Copy link
Member

Choose a reason for hiding this comment

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

Also, how about adding a jsdoc comment about possible error here

* @throws {RangeError} if `destinationArray` is provided, it must have a length of at least 2500 (`50*50`).

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the in-depth review. I'll update the branch tomorrow and let you know when it's ready for another review.

*/
getRawBuffer(destinationArray?: Uint8Array): Uint8Array;
Copy link
Member

Choose a reason for hiding this comment

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

The type of destinationArray cannot only be Uint8Array. Any TypedArray is fine. The source code in the screeps engine uses TypedArray.prototype.set to copy data into destinationArray.
(see https://github.com/screeps/engine/blob/master/src/game/rooms.js#L398 and https://github.com/screeps/engine/blob/master/src/game/rooms.js#L1237)

Btw, it's okay to copy value from uint8array to uint16array. The reverse also works

const terrainData = new Uint8Array([1, 2, 3, 4, 5]);

const uint8Receiver = new Uint8Array(5);
uint8Receiver.set(terrainData, 0); // okey
console.log(uint8Receiver); // [1, 2, 3, 4, 5]

const uint16Receiver = new Uint16Array(5);
uint16Receiver.set(terrainData, 0); //okay
console.log(uint16Receiver); // [1, 2, 3, 4, 5]

const normalReceiver = new Array(5);
Uint16Array.prototype.set.call(normalReceiver, terrainData); // error: this is not a typed array.

from uintArray16 to uintArray8

const terrainData = new Uint16Array([256,888]);

const uint8Receiver = new Uint8Array(5);
uint8Receiver.set(terrainData, 0); // okey
// (256 & ((1<<8)-1)) is 0, (888 & ((1<<8)-1)) is 120
console.log(uint8Receiver); // [ 0, 120, 0, 0, 0 ] 

const uint16Receiver = new Uint16Array(5);
uint16Receiver.set(terrainData, 0); //okay
console.log(uint16Receiver); // [ 256, 888, 0, 0, 0 ]

Copy link
Member

@DiamondMofeng DiamondMofeng Sep 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We should define a type alias of the underlying representation, then use it as the default type parameter.

Here are my suggestions for typing this method.
The first seems to be less correct. The second uses overloads, but I think it fits better.

getRawBuffer<T extends TypedArray = Uint8Array>(destinationArray?: T): T;

or

getRawBuffer<T extends TypedArray>(destinationArray: T): T;
getRawBuffer(): Uint8Array;

In case there is no built-in union for TypedArray (Big(U)Int64Array is not compatible):

type TypedArray = Int8Array | Uint8Array | Int16Array | Uint16Array | Int32Array | Uint32Array | Uint8ClampedArray | Float32Array | Float64Array;

}

interface RoomTerrainConstructor extends _Constructor<RoomTerrain> {
Expand Down