Skip to content

Conversation

@ohmissam
Copy link

No description provided.

Copy link

@Quix0r Quix0r left a comment

Choose a reason for hiding this comment

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

Final remarks: What a large PR to review and I guess you included some "personal" changes.

else if (dy === 2 && this.animated)
tileFlags |= Tile.ANIMBIT;
}
}
Copy link

Choose a reason for hiding this comment

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

To much white-spaces here.

});



Copy link

Choose a reason for hiding this comment

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

Extra empty line. Is the convention here to have only one empty between code blocks?

Budget.prototype.shouldDegradeField = function() {
return this.fieldEffect < Math.floor(15 * this.MAX_FIELD_EFFECT / 25);
};

Copy link

Choose a reason for hiding this comment

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

Extra empty line. Is the convention here to have only one empty between code blocks?


if (this.fieldMaintenanceBudget > 0)
this.fieldEffect = Math.floor(this.fieldEffect * this.fieldSpend / this.fieldMaintenanceBudget);

Copy link

Choose a reason for hiding this comment

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

This empty line wasn't there before.

fieldCost = cashRemaining;
cashRemaining -= fieldCost;


Copy link

Choose a reason for hiding this comment

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

Extra empty line. Is the convention here to have only one empty between code blocks?

</div>
</div>
<div id="tw" class="z1 rightedge initialHidden">
<a class="twitter-share-button" href="https://twitter.com/share?count=none" data-text="I'm city-building like it's 1989! Playing micropolisJS, a HTML5 retro city-builder https://graememcc.github.io/micropolisJS">Tweet</a><script type="text/javascript">window.twttr=(function(d,s,id){var t,js,fjs=d.getElementsByTagName(s)[0];if(d.getElementById(id)){return}js=d.createElement(s);js.id=id;js.src="https://platform.twitter.com/widgets.js";fjs.parentNode.insertBefore(js,fjs);return window.twttr||(t={_e:[],ready:function(f){t._e.push(f)}})}(document,"script","twitter-wjs"));</script>
Copy link

Choose a reason for hiding this comment

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

Is this wanted to remove link to author's website and Twitter/X account?

</div>
</noscript>
</main>
<footer id="footer" class="alignCenter chunk white padding10">
Copy link

Choose a reason for hiding this comment

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

Same with this. Maybe you want to keep this out of pull requests and leave it in your personal clone? You can archive this by maintaining different branches. For example develop is your local code and for a PR you create a new branch based on "upstream" (this) repository.

Exexecute: git remote add upstream https://github.com/graememcc/micropolisJS.git to add it.

<script defer src="thirdparty/jquery/jquery-2.1.1.min.js"></script>
</body>
</html>
<html><head><META HTTP-EQUIV="Content-Type" CONTENT="text/html; charset=utf-8"><meta name="Robots" content="NOINDEX " /></head><body></body>
Copy link

Choose a reason for hiding this comment

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

Second <html> tag is invalid! Please move this into existing <head> tag (without <html> of course).

"scripts": {
"build": "webpack --env.production",
"watch": "webpack --watch --env.development",
"watch": "webpack --watch --env.production",
Copy link

Choose a reason for hiding this comment

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

I wonder if this drastic change from development to production is proper?

setCropCost(POTATO_COST); break;
}
};

Copy link

Choose a reason for hiding this comment

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

Extra empty line.

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.

3 participants