From 36a6b7bd4538dada6794a59a95df5bdec3f3a8b1 Mon Sep 17 00:00:00 2001 From: ChenYunDa Date: Mon, 28 Oct 2024 16:36:27 +0800 Subject: [PATCH 1/8] refact: Optimize Rect _onValueChanged callback. --- packages/math/src/Rect.ts | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/packages/math/src/Rect.ts b/packages/math/src/Rect.ts index 415779fce4..4d71825b5a 100644 --- a/packages/math/src/Rect.ts +++ b/packages/math/src/Rect.ts @@ -1,6 +1,9 @@ import { IClone } from "./IClone"; import { ICopy } from "./ICopy"; +// Empty function for initialization of _onValueChangedCallback . +function emptyFunc(): void {} + // A 2d rectangle defined by x and y position, width and height. export class Rect implements IClone, ICopy { /** @internal */ @@ -12,8 +15,20 @@ export class Rect implements IClone, ICopy { /** @internal */ _height: number; /** @internal */ - _onValueChanged: () => void = null; - + get _onValueChanged(): () => void { + if (this._onValueChangedCallback === emptyFunc) { + return null; + } + return this._onValueChangedCallback; + } + set _onValueChanged(callback: () => void | null | undefined) { + if (callback) { + this._onValueChangedCallback = callback; + } else { + this._onValueChangedCallback = emptyFunc; + } + } + private _onValueChangedCallback: () => void = emptyFunc; /** * The x coordinate of the rectangle. */ @@ -23,7 +38,7 @@ export class Rect implements IClone, ICopy { set x(value: number) { this._x = value; - this._onValueChanged && this._onValueChanged(); + this._onValueChangedCallback(); } /** @@ -35,7 +50,7 @@ export class Rect implements IClone, ICopy { set y(value: number) { this._y = value; - this._onValueChanged && this._onValueChanged(); + this._onValueChangedCallback(); } /** @@ -47,7 +62,7 @@ export class Rect implements IClone, ICopy { set width(value: number) { this._width = value; - this._onValueChanged && this._onValueChanged(); + this._onValueChangedCallback(); } /** @@ -59,7 +74,7 @@ export class Rect implements IClone, ICopy { set height(value: number) { this._height = value; - this._onValueChanged && this._onValueChanged(); + this._onValueChangedCallback(); } /** @@ -89,7 +104,7 @@ export class Rect implements IClone, ICopy { this._y = y; this._width = width; this._height = height; - this._onValueChanged && this._onValueChanged(); + this._onValueChangedCallback(); return this; } @@ -111,7 +126,7 @@ export class Rect implements IClone, ICopy { this._y = source.y; this._width = source.width; this._height = source.height; - this._onValueChanged && this._onValueChanged(); + this._onValueChangedCallback(); return this; } } From 4cc5b864047e2e20175994ddad6ff135b1d62ec9 Mon Sep 17 00:00:00 2001 From: ChenYunDa Date: Mon, 28 Oct 2024 17:03:54 +0800 Subject: [PATCH 2/8] fixup --- packages/math/src/Rect.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/math/src/Rect.ts b/packages/math/src/Rect.ts index 4d71825b5a..fd6f5fc8f0 100644 --- a/packages/math/src/Rect.ts +++ b/packages/math/src/Rect.ts @@ -21,7 +21,7 @@ export class Rect implements IClone, ICopy { } return this._onValueChangedCallback; } - set _onValueChanged(callback: () => void | null | undefined) { + set _onValueChanged(callback: () => void | null) { if (callback) { this._onValueChangedCallback = callback; } else { From 38eea2816f6de6e79649811920fb4cc1b8bde3cb Mon Sep 17 00:00:00 2001 From: ChenYunDa Date: Mon, 28 Oct 2024 17:26:08 +0800 Subject: [PATCH 3/8] add test case --- packages/math/src/Rect.ts | 4 ++-- tests/src/math/Rect.test.ts | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/packages/math/src/Rect.ts b/packages/math/src/Rect.ts index fd6f5fc8f0..401cdeafff 100644 --- a/packages/math/src/Rect.ts +++ b/packages/math/src/Rect.ts @@ -21,8 +21,8 @@ export class Rect implements IClone, ICopy { } return this._onValueChangedCallback; } - set _onValueChanged(callback: () => void | null) { - if (callback) { + set _onValueChanged(callback: () => void | null | undefined) { + if (callback && typeof callback === 'function') { this._onValueChangedCallback = callback; } else { this._onValueChangedCallback = emptyFunc; diff --git a/tests/src/math/Rect.test.ts b/tests/src/math/Rect.test.ts index 3be1fdb019..3593e146da 100644 --- a/tests/src/math/Rect.test.ts +++ b/tests/src/math/Rect.test.ts @@ -29,4 +29,22 @@ describe("Rect test", () => { expect(a.width).to.eq(b.width); expect(a.height).to.eq(b.height); }); + it("_onValueChanged", () => { + const a = new Rect(0, 0, 1, 2); + expect(a._onValueChanged).to.eq(null); + let countChange = 0; + const _onValueChanged = () => { + countChange += 1; + }; + a._onValueChanged = _onValueChanged + expect(a._onValueChanged).to.eq(_onValueChanged); + a.x = 1; + expect(countChange).to.eq(1); + a.y = 1; + expect(countChange).to.eq(2); + a.width = 1; + expect(countChange).to.eq(3); + a._onValueChanged = null + expect(a._onValueChanged).to.eq(null); + }); }); From bd225cc47ca4fa360d9d0afe6dc9238286629ebb Mon Sep 17 00:00:00 2001 From: ChenYunDa Date: Mon, 28 Oct 2024 17:27:56 +0800 Subject: [PATCH 4/8] lint --fix --- packages/math/src/Rect.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/math/src/Rect.ts b/packages/math/src/Rect.ts index 401cdeafff..70dfda4374 100644 --- a/packages/math/src/Rect.ts +++ b/packages/math/src/Rect.ts @@ -22,7 +22,7 @@ export class Rect implements IClone, ICopy { return this._onValueChangedCallback; } set _onValueChanged(callback: () => void | null | undefined) { - if (callback && typeof callback === 'function') { + if (callback && typeof callback === "function") { this._onValueChangedCallback = callback; } else { this._onValueChangedCallback = emptyFunc; From ad9b43ac2f16bb95a74eca05126cfae308bdd26f Mon Sep 17 00:00:00 2001 From: ChenYunDa Date: Fri, 1 Nov 2024 16:59:19 +0800 Subject: [PATCH 5/8] add test case --- tests/src/math/Rect.test.ts | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/src/math/Rect.test.ts b/tests/src/math/Rect.test.ts index 3593e146da..ee25571cdf 100644 --- a/tests/src/math/Rect.test.ts +++ b/tests/src/math/Rect.test.ts @@ -1,5 +1,6 @@ import { Rect } from "@galacean/engine-math"; import { expect } from "chai"; +import { count } from "console"; describe("Rect test", () => { it("set", () => { @@ -47,4 +48,40 @@ describe("Rect test", () => { a._onValueChanged = null expect(a._onValueChanged).to.eq(null); }); + it("x", () => { + let countChange = 0; + const _onValueChanged = () => { + countChange += 1; + }; + const a = new Rect(0, 0, 1, 2); + a._onValueChanged = _onValueChanged + a.x = 1; + expect(a.x).to.eq(1); + expect(countChange).to.eq(1); + a.x = 2; + expect(a.x).to.eq(1); + expect(countChange).to.eq(2); + }); + it("y", () => { + let countChange = 0; + const _onValueChanged = () => { + countChange += 1; + }; + const a = new Rect(0, 0, 1, 2); + a._onValueChanged = _onValueChanged + a.y = 1; + expect(countChange).to.eq(1); + expect(a.y).to.eq(1); + expect(countChange).to.eq(2); + }); + it("width", () => { + const a = new Rect(0, 0, 1, 2); + a.width = 1; + expect(a.width).to.eq(1); + }); + it("height", () => { + const a = new Rect(0, 0, 1, 2); + a.height = 1; + expect(a.height).to.eq(1); + }); }); From d49da11bf1be985fed660203640b48652831283a Mon Sep 17 00:00:00 2001 From: ChenYunDa Date: Fri, 1 Nov 2024 17:03:07 +0800 Subject: [PATCH 6/8] fix test case --- tests/src/math/Rect.test.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/src/math/Rect.test.ts b/tests/src/math/Rect.test.ts index ee25571cdf..2a697df00c 100644 --- a/tests/src/math/Rect.test.ts +++ b/tests/src/math/Rect.test.ts @@ -55,11 +55,11 @@ describe("Rect test", () => { }; const a = new Rect(0, 0, 1, 2); a._onValueChanged = _onValueChanged - a.x = 1; - expect(a.x).to.eq(1); + a.x = 9; + expect(a.x).to.eq(9); expect(countChange).to.eq(1); - a.x = 2; - expect(a.x).to.eq(1); + a.x = 10; + expect(a.x).to.eq(10); expect(countChange).to.eq(2); }); it("y", () => { @@ -72,16 +72,18 @@ describe("Rect test", () => { a.y = 1; expect(countChange).to.eq(1); expect(a.y).to.eq(1); + a.y = 2 + expect(a.y).to.eq(2); expect(countChange).to.eq(2); }); it("width", () => { const a = new Rect(0, 0, 1, 2); - a.width = 1; - expect(a.width).to.eq(1); + a.width = 5; + expect(a.width).to.eq(5); }); it("height", () => { const a = new Rect(0, 0, 1, 2); - a.height = 1; - expect(a.height).to.eq(1); + a.height = 6; + expect(a.height).to.eq(6); }); }); From 3456275f185d30f113c7b49086b71b41632a9602 Mon Sep 17 00:00:00 2001 From: ChenYunDa Date: Fri, 1 Nov 2024 17:07:06 +0800 Subject: [PATCH 7/8] update test case --- tests/src/math/Rect.test.ts | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/src/math/Rect.test.ts b/tests/src/math/Rect.test.ts index 2a697df00c..1ff1edda33 100644 --- a/tests/src/math/Rect.test.ts +++ b/tests/src/math/Rect.test.ts @@ -63,11 +63,24 @@ describe("Rect test", () => { expect(countChange).to.eq(2); }); it("y", () => { + const a = new Rect(0, 0, 1, 2); + a._onValueChanged = undefined; + expect(a._onValueChanged).to.eq(null); + // Test non-function value + a._onValueChanged = "not a function" as any; + expect(a._onValueChanged).to.eq(null); + + // Test multiple assignments + const callback1 = () => {}; + const callback2 = () => {}; + a._onValueChanged = callback1; + expect(a._onValueChanged).to.eq(callback1); + a._onValueChanged = callback2; + expect(a._onValueChanged).to.eq(callback2); let countChange = 0; const _onValueChanged = () => { countChange += 1; }; - const a = new Rect(0, 0, 1, 2); a._onValueChanged = _onValueChanged a.y = 1; expect(countChange).to.eq(1); From ca3f4f43cbae385a3efadf71f1aa0b0f6710c83a Mon Sep 17 00:00:00 2001 From: ChenYunDa Date: Fri, 1 Nov 2024 17:07:42 +0800 Subject: [PATCH 8/8] remove unused package --- tests/src/math/Rect.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/src/math/Rect.test.ts b/tests/src/math/Rect.test.ts index 1ff1edda33..34ec869206 100644 --- a/tests/src/math/Rect.test.ts +++ b/tests/src/math/Rect.test.ts @@ -1,6 +1,5 @@ import { Rect } from "@galacean/engine-math"; import { expect } from "chai"; -import { count } from "console"; describe("Rect test", () => { it("set", () => {