Add some high level review comments

.env.example

@@ -0,0 +1,3 @@
+# Having an example .env file is useful because you can commit all of the expected environment
+# variable names. That way if you have to install this project on a new machine you'll
+# have a better idea of everything needed in order to run it.

ormconfig.js

@@ -17,6 +17,8 @@ module.exports = {
 	namingStrategy: new SnakeNamingStrategy(),
 	entities: [rootDir + '/entity/**/*{.ts,.js}'],
 	subscribers: [rootDir + '/subscriber/**/*{.ts,.js}'],
+	// You should really be using migrations to handle all of your schema
+	// updates as opposed to letting typeorm handle the synchronization.
 	migrations: [rootDir + '/migrations/**/*{.ts,.js}'],
 	cli: {
 		entitiesDir: rootDir + '/entity',

src/config/races.ts

@@ -7,7 +7,10 @@
 // const orc = 6
 // const drow = 7
 // const goblin = 8
-
+/*
+	This is another thing that I think would probably be better as a database table
+	instead of being hardcoded here.
+*/
 const RACE_HUMAN = {
 	name: 'Human',
 	mod_offense: 0,

src/index.ts

@@ -103,7 +103,11 @@ app.get('/api/perpetual/hello', (req, res) => {
 	console.log(req.url)
 	res.send('hello perpetual')
 })
-
+/*
+	In my earlier PR I created and index file in your routes directory that exported a single router
+	to be consumed here as `app.use('/api', apiRoutes)` -- I'd recommend following that pattern
+	and moving all of these individual routes from here into that index file.
+*/
 app.use('/api/auth', authRoutes)
 app.use('/api/empire', empireRoutes)
 app.use('/api/useturns', useTurns)
@@ -134,6 +138,10 @@ app.get('/debug-sentry', function mainHandler(req, res) {
 // The error handler must be registered before any other error middleware and after all controllers
 app.use(Sentry.Handlers.errorHandler())
 
+/*
+	I'd move this error handler and the Sentry initialization call to their own file
+	that you then import at the top of this one.
+*/
 // Optional fallthrough error handler
 app.use(function onError(err, req, res, next) {
 	// The error id is attached to `res.sentry` to be returned
@@ -164,6 +172,9 @@ function checkTime() {
 	} else return false
 }
 
+/*
+	I'd move all of this scheduler logic into a separate file.
+*/
 if (process.env.NODE_ENV === 'development') {
 	let gameOn = false
 

src/routes/actions/achievements.ts

@@ -1,6 +1,10 @@
 import Empire from '../../entity/Empire'
 import { achievements } from '../../config/achievements'
 
+/*
+	You've got a copy of this method that also lives in the EmpireSubscriber so it's
+	worth moving it to the utils directory and then importing it from there.
+*/
 function sortObject(obj: Record<string, unknown>) {
 	return Object.keys(obj)
 		.sort()

src/routes/attack.ts

@@ -918,7 +918,28 @@ const attack = async (req: Request, res: Response) => {
 				// won = true
 				let buildLoss: buildLoss = {}
 				let buildGain: buildGain = {}
-
+				/*
+					One big general note is to try and keep your files on the smaller side
+					(think a couple hundred lines on the high end).
+					Not sure if you've heard of the DRY acronym when it comes to coding, but it stands for Don't Repeat Yourself.
+					It's a pretty easy thing to look out for that can help keep files to more manageable sizes.
+					The destroyBuildings calls you have below are a decent example of mostly repeated code that can be shortened
+					up with a pretty simple refactor. The only things that change from call to call are arguments 2-4 so I'd suggest
+					putting them in an array and then iterating through it and calling destroyBuildings for each item.
+
+					It would look something like:
+
+					const destroyBuildingsParams = [
+						{ pcloss: 0.07 * lowLand, pcgain: 0.7 * lowLand, type: 'bldCash' },
+						{ pcloss: 0.07 * lowLand, pcgain: 0.7 * lowLand, type: 'bldPop' },
+						{ pcloss: 0.07 * lowLand, pcgain: 0.5 * lowLand, type: 'bldTroop' },
+						....
+					]
+
+					destroyBuildingParams.forEach(({ pcloss, pcgain, type }) => {
+						destroyBuildings(attackType, pcloss, pcgain, type, defender, attacker, buildLoss, buildGain)
+					})
+				*/
 				destroyBuildings(
 					attackType,
 					0.07 * lowLand,

src/routes/spells/advance.ts

@@ -1,3 +1,13 @@
+/*
+	I would suggest moving the `spells` and `actions` directories out of the routes directory and
+	maybe into something like a `services` directory.
+	I'd also try to keep the files that are in the routes directory mapped to the route path
+	that it's responsible for.
+	For example:
+	`routes/admin.ts` is responsible for handling calls to `/api/admin`
+	Doing it this way creates a nice paradigm for knowing what your route tree
+	looks like just by perusing your file system.
+*/
 import { eraArray } from '../../config/eras'
 import Empire from '../../entity/Empire'
 import EmpireEffect from '../../entity/EmpireEffect'

src/util/helpers.ts

@@ -1,3 +1,11 @@
+/*
+	Both makeId and slugify are examples of things I think are better to leverage a library for.
+	https://www.npmjs.com/package/uuid can be used for generating IDs.
+	Or you can also use the native `crypto.randomUUID()` if you're only using the v4
+	uuid generator (which is all I've ever used).
+	As for slugify I would recommend using lodash: https://www.npmjs.com/package/lodash
+	It has a ton of useful utility methods and one of them is `kebabCase` which is equivalent to your slugify.
+*/
 export function makeId(length: number): string {
 	let result = ''
 	const characters =