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.

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

updated jshint added "unused": true #363

Closed

Conversation

mschultheiss83
Copy link
Contributor

fixed issues
fixed similar code: the roles.[role].died function
fixed complexity for brain.handleUnexpectedDeadCreeps by extracting functions

  fixed issues
  fixed similar code: the `roles.[role].died` function
  fixed complexity for brain.handleUnexpectedDeadCreeps by extracting functions
@mschultheiss83
Copy link
Contributor Author

@TooAngel @Nevrdid @Somotaw some reviews please, i know many changes in here, but if you're using some inspection there are not sooo many,
with the JSHint Option "unused" you will find them realy fast

Copy link
Contributor

@Nevrdid Nevrdid left a comment

Choose a reason for hiding this comment

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

The only thing i seen which may need a fix is call to executeEveryTick wrongly written (didn't verify but else, the method should be renamed ^^'). About returnCode, i think we need a good return policy until this we can remove all the ones you commented.

@@ -120,7 +120,7 @@ Creep.prototype.handleExtractor = function() {
if (this.carry[key] === 0) {
continue;
}
let returnCode = this.transfer(this.room.terminal, key);
returnCode = this.transfer(this.room.terminal, key);
Copy link
Contributor

Choose a reason for hiding this comment

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

may comment returncode here too.

@@ -13,7 +13,7 @@ Creep.prototype.getRoute = function() {

// Add room avoidance
let route = [];
let creep = this;
//let creep = this;
Copy link
Contributor

Choose a reason for hiding this comment

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

can remove it totally

@@ -159,7 +159,7 @@ Creep.prototype.getEnergyFromStorage = function() {

Creep.prototype.repairStructure = function() {
let structure = null;
let i = null;
//let i = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

free to remove

creep: 2,
role: 'squadheal'
}
];
Copy link
Contributor

Choose a reason for hiding this comment

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

are you sure about that ?

Copy link
Owner

Choose a reason for hiding this comment

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

I don't think this is correct, line 24 can be removed

};
//let getWalls = function(object) {
// return object.structureType === STRUCTURE_WALL;
//};
Copy link
Contributor

Choose a reason for hiding this comment

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

we now use propertyfilters. can be remove so i supose

creep: {}
};
creep: {}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

still strange for me x)

@@ -43,13 +43,13 @@ Room.prototype.checkExitsAreReachable = function() {

let exits = this.find(FIND_EXIT);
let room = this;
var callbackNew = function(roomName) {
var callbackNew = function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

return room.getMemoryCostMatrix() for line under ? ^^'

let costMatrix = room.getMemoryCostMatrix();
return costMatrix;
};
for (let exit of exits) {
// console.log(exit);
let room = this;
//let room = this;
Copy link
Contributor

Choose a reason for hiding this comment

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

free to remove


roles.autoattackmelee.preMove = function(creep) {
// creep.log('!!!!!!!!!!!!!!!! Autoattacking');
if (creep.room.exectueEveryTicks(25)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

exectue ?

Copy link
Owner

Choose a reason for hiding this comment

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

Well, it is correct.

We should rename the method ...

Copy link
Contributor

Choose a reason for hiding this comment

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

^^' didn't verify

};
//roles.squadheal.execute = function(creep) {
// creep.log('Execute!!!');
//};
Copy link
Contributor

Choose a reason for hiding this comment

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

need tooAngel validation but pretty sure it can be totally removed. I remember i removed some call in room_my if i'm right

Copy link
Owner

Choose a reason for hiding this comment

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

Hm, not sure :-)

I think we should get rid of the execute function completely. This was the old system for executing creeps, if action returns false, the main loop will execute execute, this shouldn't happen anymore.

Copy link
Owner

@TooAngel TooAngel left a comment

Choose a reason for hiding this comment

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

I like, especially enforcing it for the future.

IMHO all old code parts can be removed instead of comments.
We have version control if we want to check.

@@ -111,7 +132,7 @@ brain.cleanRooms = function() {
if (Game.time % 300 === 0) {
for (let name in Memory.rooms) {
// Check for reserved rooms
let memory = Memory.rooms[name];
//let memory = Memory.rooms[name];
Copy link
Owner

Choose a reason for hiding this comment

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

Can be removed

@@ -213,7 +213,7 @@ Creep.prototype.buildRoad = function() {
return false;
}

var i;
//var i;
Copy link
Owner

Choose a reason for hiding this comment

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

Can be removed

creep: 2,
role: 'squadheal'
}
];
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think this is correct, line 24 can be removed

@@ -1,12 +1,12 @@
Room.prototype.getFriends = function(object) {
Room.prototype.getFriends = function() {
Copy link
Owner

Choose a reason for hiding this comment

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

^^ I don't understand that comment


roles.autoattackmelee.preMove = function(creep) {
// creep.log('!!!!!!!!!!!!!!!! Autoattacking');
if (creep.room.exectueEveryTicks(25)) {
Copy link
Owner

Choose a reason for hiding this comment

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

Well, it is correct.

We should rename the method ...

};
//roles.squadheal.execute = function(creep) {
// creep.log('Execute!!!');
//};
Copy link
Owner

Choose a reason for hiding this comment

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

Hm, not sure :-)

I think we should get rid of the execute function completely. This was the old system for executing creeps, if action returns false, the main loop will execute execute, this shouldn't happen anymore.

@ghost
Copy link

ghost commented Aug 16, 2017

A new review, yeah.

Votes: 248/463
Coefficient: 0.535637149028
Merging in 6 days 18 hours
Age 0 days 4 hours

@TooAngel
Copy link
Owner

TooAngel commented Sep 4, 2017

Would like to see this merged into master, mind fixing the conflicts?

@mschultheiss83
Copy link
Contributor Author

#411

@mschultheiss83 mschultheiss83 deleted the feature/unused-var branch September 22, 2017 10:08
@mschultheiss83 mschultheiss83 restored the feature/unused-var branch September 22, 2017 10:08
@mschultheiss83 mschultheiss83 deleted the feature/unused-var branch February 25, 2023 15:06
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.

None yet

3 participants