Add.joysticks #12

sebferrer · 2018-10-07T15:51:48Z

Add joysticks for mobile / Android: left for moving, right for attacks

(without this.key_up() trigging)

(stupid copy/paste mistake)

sebferrer · 2018-10-08T21:04:38Z

src/gamestate.ts

 	}

-	public key_down(keyName: string): void {
+	public touch_start(touches): void {


…types

bastienguillon · 2018-10-08T22:20:05Z

src/touch_helper.ts

+		const touches_array = new Array<Touch>();
+		if(touchlist != null) {
+			for (let i = 0; i < touchlist.length; i++) {
+				touches_array.push(touchlist[i]);


Sur la doc de TouchList, ils disent que .item(x) permet de récupérer l'objet Touch à l'index x de la TouchList, je ne sais pas si ça change quelque chose par rapport aux indexers [x]

cf. https://developer.mozilla.org/en-US/docs/Web/API/TouchList/item

Sinon, pour simplifier la création du tableau, tu devrais pouvoir utiliser la méthode static Array.from(touchlist) (à tester)

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/from

.item(x) : je regarde ça
Array.from() : déjà essayé, mais TouchList ne prend aucune fonction relavite aux Arrays

bastienguillon · 2018-10-08T22:29:13Z

src/gamestate.ts

@@ -21,9 +22,10 @@ export class GameState {
 	public directions_keyDown: Direction[];
 	public attack_direction_event: AttackDirectionEvent;
 	public tears: Tear[];
+	public joysticks: Joystick[];


Comme on en discutait, le type de joysticks pourrait simplement devenir { left: Joystick, right: Joystick }, ça permettrait d'éviter de faire un find pour savoir si tel ou tel Joystick est présent ou non (même si c'est bien bien grave dans la mesure où c'est fait uniquement dans touch_start)

J'y avais pensé dès le début, mais dans tous les cas on aurait besoin de les mettre dans un array pour itérer dessus dans le move().
Quoique je viens d'avoir une idée potentiellement cool, tu verras au prochain commit

bastienguillon · 2018-10-08T22:30:29Z

src/main.ts

@@ -4,6 +4,9 @@ import { GameState } from "./gamestate";
 import { Settings } from "./settings/settings";
 import { KeyMapper } from "./settings/keymapper";

+export const window_W = window.innerWidth;


Pourquoi ne pas utiliser directement window.innerWidth ? En utilisant une variable comme ça, tu figes la largeur de la fenêtre dans le programme, ce qui empêche de gérer une éventuelle redimension

C'était plus un réflexe, je n'avais pas envisagé le redimensionnement, merci

bastienguillon · 2018-10-08T22:39:11Z

src/util.ts

 		if (!array.includes(obj)) {
 			array.unshift(obj);
 			return true;
 		}
 		return false;
 	}
+
+	public static diff(array1: Array<any>, array2: Array<any>): Array<any> {


Sachant que tu utilises cette méthode avec n'importe quel type d'objet, tu es sûr que ça fait ce que tu veux ? Un test simple :

Avec des types références

var a = [ { test: 1 }, { test: 2 }, { test: 3 }]; a.indexOf({ test: 1 }); // -1

Avec des types valeur

var b = [1, 2, 3, 4]; b.indexOf(3); // 2

Sur les collections de value type (number, string, etc), indexOf va tester utiliser l'égalité pour savoir où se trouve l'item demandé dans le tableau

Sur les collections de reference type, indexOf va utiliser l'égalité de la référence de l'objet passé en paramètre aux références des items du tableau

Bref, indexOf utilise une égalité stricte

Tu es sûr que les deux tableaux contiennent bien des références d'objets communes quand tu appelles ta méthode ?

A priori les références d'objets Touches qui sont manipulées dans le gamestate ne changent pas, c'est juste pour être sûr

Sinon, ta méthode pourrait devenir générique, ce serait mieux pour avoir des infos sur les types quand tu appelles ta méthode

public static diff2<T>(array1: Array<T>, array2: Array<T>): Array<T> { return array1.filter(item => array2.indexOf(item) < 0); }

Pour les premières remarques, c'est effectivement ce que je cherchais à faire ;)
Dernière remarque : effectivement c'est pas mal

bastienguillon · 2018-10-08T22:51:44Z

src/gamestate.ts

+
+		const joysticks_to_remove = new Array<Joystick>();
+
+		touches_removed.forEach(touch_identifier => {


Une version one-liner :

const joysticks_to_remove = this.joysticks.filter(js => touches_removed.indexOf(js.touch_identifier) > -1);

Non merci, je préfère quand y'a plus de lignes.
(Je déconne hein)

bastienguillon · 2018-10-08T23:14:35Z

src/gamestate.ts

@@ -119,6 +244,8 @@ export class GameState {

 		this.jays.draw(ctx);

+		this.touch_move();


Je sais pas trop si touch_move a vraiment besoin d'être appelé aussi souvent que le jeu n'est refresh, d'autant plus qu'on refresh le jeu aussi souvent que le navigateur le permet en utilisant requestAnimationFrame... Faudrait peut-être ajouter une gestion des FPS d'ailleurs ?

Comme dis hier du coup, je l'avais effectivement fait, et non seulement ça ralentissait trop les joysticks, mais en plus le lag était toujours là :(
J'ai l'impression que le lag vient du call d'event lui-même, j'sais pas trop encore

bastienguillon · 2018-10-08T23:26:38Z

src/joystick.ts

+		this.force.y = y - this.center.y;
+		this.next_circle.radius = this.controller.circle.radius;
+		if (this.zone.circle.containsPoint(this.next_circle.pos)) {
+			const angle = this.zone.circle.pos.angle(this.next_circle.pos) + (Math.PI / 2);


une micro optimisation un peu pourrie : stocker le résultat de Math.Pi / 2 dans une variable static pour éviter de le calculer à chaque fois

Pourquoi pas, même si je pense que le problème est tellement ailleurs que ça ne changera rien

bastienguillon · 2018-10-08T23:28:02Z

src/joystick.ts

+			this.next_circle.pos = this.next_circle.pos.translateFromPoint(angle, this.zone.circle.radius, this.zone.circle.pos);
+		}
+
+		this.coeff_x = (this.zone.circle.pos.x - this.next_circle.pos.x) * (-1) / this.zone.circle.radius;


Ecrire (this.next_circle.pos.x - this.zone.circle.pos.x) plutôt que (this.zone.circle.pos.x - this.next_circle.pos.x) permettrait d'éviter le * - 1

/hackerman

bastienguillon · 2018-10-08T23:31:05Z

src/joystick.ts

+		this.coeff_x = (this.zone.circle.pos.x - this.next_circle.pos.x) * (-1) / this.zone.circle.radius;
+		this.coeff_x = Math.round(this.coeff_x * 100) / 100;
+		this.coeff_y = (this.zone.circle.pos.y - this.next_circle.pos.y) / this.zone.circle.radius;
+		this.coeff_y = Math.round(this.coeff_y * 100) / 100;


Alors ça peut paraître couillon, mais dans l'idée d'améliorer les perfs, travailler avec des valeurs entières plutôt que des flottants pourrait aider

Ce serait faisable de se passer de nombres à virgules dans tous ces calculs, en utilisant de grands nombres entiers ? J'ai déjà entendu parler de trucs dans ce genre à l'IUT

J'suis pas sûr de comprendre ce que tu veux faire. Le soucis c'est que les valeurs de base récupérées sont déjà flottantes...

bastienguillon · 2018-10-08T23:33:59Z

src/joystick.ts

+		div.style.left = rect.x + "px";
+		div.style.top = rect.y + "px";
+		div.style.backgroundColor = color;
+		div.style.borderRadius = "20em";


Pas très important ici, je l'ai appris récemment, mais l'unité css rem est meilleure que em. L'unité em dépend de la taille de la police de l'élément concerné, ce qui n'est pas le cas de rem qui possède une valeur "absolue" (d'après ce que j'ai compris)

Bon à savoir, je jetterai un oeil pour être sûr.
En l'occurence là on s'en tape c'est juste une valeur grossièrement élevée pour faire un cercle

…) + code clean

sebferrer · 2018-10-13T22:27:48Z

src/util.ts

-	public static add_first_no_duplicate(array: any, obj: any): boolean {
-		if (!array.includes(obj)) {
+	public static add_first_no_duplicate<T>(array: Array<T>, obj: T): boolean {
+		if (!(array.indexOf(obj) >= 0)) {
 			array.unshift(obj);


includes n'est compatible avec les arrays génériques qu'à partir d'es2016

D'ailleurs, je ne savais même pas qu'on pouvait faire des types génériques en JS de la même façon qu'en Java, c'est grâce à l'un de tes derniers coms. Nous voilà débarrassé des derniers any !

Cool, je pourrais rajouter une règle dans la config du linter pour "empêcher" l'utilisation du type any si tu veux

Yep !
C'est d'ailleurs ce qu'on avait prévu depuis le début, mais on s'était autorisés à utiliser le any au début, le temps de capter tous les types. Puis ensuite c'était moi qui bloquait avec ces fonctions génériques. On est enfin bons !

sebferrer · 2018-10-13T22:30:14Z

src/joystick.ts


-	constructor(id: string, zonePos: Point, zoneRadius: number, controllerPos: Point, controllerRadius: number, touch_identifier: number) {
+	constructor(id: string, zonePos: Point, zoneRadius: number, controllerPos: Point, controllerRadius: number, touch_identifier: number, remove_events_callback: Function) {


Plus pratique pour éviter de chercher quel Joystick doit faire quoi au moment du touch_end, je leur dis quoi faire dès leur création

sebferrer · 2018-10-13T22:31:29Z

src/gamestate.ts

-					this.remove_attack_event(Direction.UP);
-					break;
-			}
+			joystick.destroy();


delete ne fonctionne que sur un attribut d'objet, pas sur l'objet lui-même. J'ai au passage vu à plusieurs endroits que mettre explicitement un objet à null pour le GC était un mythe d'optimisation, le GC se débrouille déjà très bien

sebferrer · 2018-10-13T22:32:30Z

src/gamestate.ts

+		}
+		if (this.joysticks.right != null) {
+			current_joysticks.push(this.joysticks.right);
+		}


C'est pas magnifique mais ça permet d'utiliser le filter efficacement

Pour éviter les deux conditions :

const joysticks_to_remove = [this.joysticks.left, this.joysticks.right].filter(j => j != null && touches_removed.indexOf(j.touch_identifier) > -1);

Ah, j'avais eu exactement la même idée, mais j'ai pas pensé à mettre la condition sur null dans le filter, c'est pour ça que j'étais revenu à ces conditions de merde, merci

sebferrer · 2018-10-13T22:34:02Z

src/touch_helper.ts

@@ -21,8 +21,8 @@ export class TouchHelper {

 	public static get_touch_by_identifier(touchlist: TouchList, identifier: number): Touch {
 		for(let i = 0; i < touchlist.length; i++) {
-			if(touchlist[i].identifier === identifier) {
-				return touchlist[i];
+			if(touchlist.item(i).identifier === identifier) {


J'ai pas trouvé la différence entre les deux. Il y a de grandes chances qu'il y ait simplement une surcharge de l'opérateur [], appelant simplement item()...

Possible en effet !

implementation

bastienguillon · 2018-10-13T23:06:51Z

src/character/jays.ts

@@ -79,7 +79,7 @@ export class Jays extends Entity implements IDrawable {
 	}

 	public direction_key_up(direction: Direction): void {
-		if (gameState.directions_keyDown.length > 0) {
+		if (gameState.directions_keyDown.size > 0) {


Ca leur aurait vraiment trouvé le cul d'appeler ça length hein

Ahah j'ai pensé la même chose.
Après je m'en doutais, c'est la même logique de différence entre les arrays et les lists en Java...

bastienguillon · 2018-10-13T23:13:48Z

src/joysticks.ts

+		this._right = null;
+	}
+
+	public get left() {


Perso j'aime bien mettre les accesseurs juste à côté de leurs attributs privés, on s'y retrouve mieux, mais c'est pas bien grave

Consistence est mère de sûreté
Prudence est mère de famille !

bastienguillon · 2018-10-13T23:20:20Z

src/joystick.ts

@@ -16,8 +16,9 @@ export class Joystick {
 	public next_circle: Circle;
 	public force: Point;
 	public touch_identifier: number;
+	public remove_events_callback: Function;


J'ai jamais utilisé le type Function, j'ai plus l'habitude d'utiliser la notation public remove_events_callback: () => void mais ok

bastienguillon · 2018-10-13T23:25:06Z

src/gamestate.ts

-			}
+			joystick.destroy();
+			joystick.remove_events_callback();
+			this.joysticks.remove(joystick);


Peut-être appeler destroy après avoir appelé remove_events_callback ? ça me semble plus logique non ?

Sinon avec une boucle for tu pourrais faire un delete à priori:

for(let i = 0; i < joysticks_to_remove.length; ++i) { joysticks_to_remove[i].remove_events_callback(); joysticks_to_remove[i].destroy(); delete joysticks_to_remove[i]; }

Ça m'a l'air un peu bizarre les conditions pour utiliser delete.
Mais je prends

sebferrer added 12 commits September 17, 2018 01:57

[WIP] Add joysticks. TODO remove lag, differentiate the sides

2316c4a

[WIP] Multi-touch joysticks optimization, still trying...

3a77a04

Merge branch 'generate.maps' into add.joysticks

45f0443

Multi event types + optimizations

efb2caf

Multi-josticks feature successfully integrated

2e79228

[WIP] Bind joysticks with Jays actions. TODO touch_end issue

d7aba57

Touch events binding with joysticks. TODO bind with events properly

e057386

(without this.key_up() trigging)

Fix animation bug

d1497b2

(stupid copy/paste mistake)

Way better method to track joysticks to delete on touch_end

f6a6eae

Merge branch 'master' into add.joysticks

e1c1b53

Better organization of event arrays + real joysicks - events binding

c8362f1

Mere omission

ab06e20

sebferrer commented Oct 8, 2018

View reviewed changes

src/gamestate.ts Outdated

}

public key_down(keyName: string): void {

public touch_start(touches): void {

Copy link

Owner Author

sebferrer Oct 8, 2018

Choose a reason for hiding this comment

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

Type

Touch & TouchList functions into TouchHelper + forgetfulness of some …

d08749f

…types

bastienguillon requested changes Oct 8, 2018

View reviewed changes

Replace array of Joysticks by Joysticks object (this.left, this.right…

61ad810

…) + code clean

sebferrer commented Oct 13, 2018

View reviewed changes

directions_keyDown Array replaced by a Set + basic Set methods

c442ecc

implementation

bastienguillon reviewed Oct 13, 2018

View reviewed changes

Organize imports

4d08702

bastienguillon reviewed Oct 13, 2018

View reviewed changes

Minor corrections (organization, types, ...)

5d5f4f2

bastienguillon approved these changes Oct 13, 2018

View reviewed changes

bastienguillon merged commit f9e03b8 into master Oct 13, 2018

bastienguillon deleted the add.joysticks branch October 13, 2018 23:42


		const joysticks_to_remove = new Array<Joystick>();

		touches_removed.forEach(touch_identifier => {

		@@ -119,6 +244,8 @@ export class GameState {

		this.jays.draw(ctx);

		this.touch_move();


		constructor(id: string, zonePos: Point, zoneRadius: number, controllerPos: Point, controllerRadius: number, touch_identifier: number) {
		constructor(id: string, zonePos: Point, zoneRadius: number, controllerPos: Point, controllerRadius: number, touch_identifier: number, remove_events_callback: Function) {

Add.joysticks #12

Add.joysticks #12

Conversation

sebferrer commented Oct 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bastienguillon Oct 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bastienguillon Oct 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bastienguillon Oct 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bastienguillon Oct 8, 2018 •

edited

Loading

bastienguillon Oct 13, 2018 •

edited

Loading

bastienguillon Oct 13, 2018 •

edited

Loading