Skip to content
This repository has been archived by the owner on Sep 3, 2022. It is now read-only.

Resolve #43 -- Code rework #44

Closed
wants to merge 3 commits into from
Closed

Resolve #43 -- Code rework #44

wants to merge 3 commits into from

Conversation

Deudly
Copy link
Member

@Deudly Deudly commented Aug 14, 2017

Code rework, new design and summoner icons

Refs #43

Deudly added 2 commits August 14, 2017 14:36
Code rework and new design

Refs #43
Copy link
Member

@molenzwiebel molenzwiebel left a comment

Choose a reason for hiding this comment

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

I didn't review your styles since you are wildly inconsistent with how you format your less files. Mixed indentation, spaces before commas and no spaces before commas, lines over 300 characters long just to mention a few things. I recommend you get a proper editor to work on this project, since you're obviously not using one (or you're not using it properly).

messages: [],
lobby: NetworkServiceStatic.currentLobby,
network: NetworkService,
isHost: this.isHost,
Copy link
Member

Choose a reason for hiding this comment

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

This won't do anything.

get playerSettings() {
return this.settings.playerSettings;

iconByUser(user: { idServer: number}) {
Copy link
Member

Choose a reason for hiding this comment

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

iconForUser would be a better name, fix the spacing, get a complete type in here.

Copy link
Member Author

Choose a reason for hiding this comment

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

d

}
}

data() {
this.canCreateLobby = true;
Copy link
Member

Choose a reason for hiding this comment

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

This should go in the data initializer.

Copy link
Member Author

Choose a reason for hiding this comment

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

d

network: NetworkService
network: NetworkService,
staticService: StaticService,
lobbySelected: undefined,
Copy link
Member

Choose a reason for hiding this comment

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

This should be null, not undefined.

Copy link
Member Author

Choose a reason for hiding this comment

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

d

};
}

select(lobby: lobby.LobbyListItem) {
this.lobbySelected = lobby;
Copy link
Member

Choose a reason for hiding this comment

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

selectedLobby would be a better name.

Copy link
Member Author

Choose a reason for hiding this comment

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

d

});
function startGame(gameServerPort){
function startGame(data){
Copy link
Member

Choose a reason for hiding this comment

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

Nested functions are never a good idea.

Copy link
Member Author

Choose a reason for hiding this comment

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

d

execFile.execFile(configContent.pathToLolExe,
args, {cwd: configContent.pathToLolFolder, maxBuffer: 1024 * 90000},
execFile(localStorage.getItem("path") + "/League of Legends.exe",
args, {cwd: localStorage.getItem("path"), maxBuffer: 1024 * 90000},
Copy link
Member

Choose a reason for hiding this comment

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

Either leave this on a single line or indent it properly, this is one statement that looks like two.

Copy link
Member Author

Choose a reason for hiding this comment

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

d

this.emit("host", true);
} else {
this.emit("host", false);
}
Copy link
Member

Choose a reason for hiding this comment

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

this.emit("host", data.isHost)

Copy link
Member Author

Choose a reason for hiding this comment

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

d

@@ -153,7 +153,7 @@ declare namespace lobby {
/** Name of the lobby. */
name: string;
/** Creator of the lobby. */
Copy link
Member

Choose a reason for hiding this comment

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

Update the documentation.

Copy link
Member Author

Choose a reason for hiding this comment

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

d

id: number;

iconURL: string;
}
Copy link
Member

Choose a reason for hiding this comment

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

Documentation.

Copy link
Member Author

Choose a reason for hiding this comment

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

d

@Deudly Deudly closed this Nov 29, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants