POB and player monitoring/alerting bot for Discord

[dr lameos]

It tells me that my POB does not exist

It needs to have the exact same name as on the POB status page, and is case sensitive

im in 2 different discords, could this bot work to just show me when im in the other one not the disco one?

Not sure I understand fully - Your Pobbot configuration is specific to the server channel you select. So add Pobbot the the server/channel you want alerts on, and don't add it to the one you don't want alerts on?

Okay, this is going to be a big post. Hope you like feedback. I'll preface this with saying that this analysis is from a skim read of your project, and is my no means an in-depth review. (Judging by your code, I'd say with a degree of certainty, you were following this tutorial)

I think I started with this one:
https://www.digitaltrends.com/gaming/how...scord-bot/

Key notes:
You have a single js file that's almost 600 lines and is running in a err... linear fashion. Consider moving your events to their own files and then having a single startup file. Use module.exports or export default function for the events and then move the other functions to an independent file.
- You are requiring fs every time you use it. This is a bad practice.

I think what I generally haven’t worked out yet is how to pass variables to a function - leading to them being nested, repeated or using global vars

- You are importing winston for logging, which isn't a bad idea, but you declare it, use it once, then revert to using console.log and console.error throughout the rest of your code.
- You have fs listed in your package.json - don't do this. It's been part of Node since 8.0 iirc.

I’ll correct this.

- You are storing everything inside of json files and writing to them constantly. This is a terrible idea, and there are numerious articles on the internet on why you shouldn't use json for persistence (config files are fine, obviously).

In effect my json is a configuration file - originally it was static - then I made it editable through commands. It could be moved to a database, but seems a bit unnecessary at this point.

- We are living in 2019 but your code is acting like it's 2012. Use modern syntax. E.g. replace var with let/const, use arrow functions, etc.

That makes sense, last time I was involved in any code it was 2011!

- You are using extremely nested if functions, inside of loops. Use break, continue, and return statements for better control.
- You are chaining a lot of "if {} else if {} else if {} else if {}" etc. I would suggest using switch case statements instead.

I know, it could be a lot cleaner. It's gone through 5 large revisions so far and I'm intending to do more. However I've decided to release it at this point, as it's working and has the necessary functionality

- You have unused variables declared throughout your code.

Can you give an example? I can only see the one (page)

- You are sending messages to Discord using a deprecated method (sendMessage had support removed like two versions ago). Use msg.channel.send instead.

Thanks, I'll blame that on the tutorial

- Your code is seriously lacking formatting in many places. Malformed indents (sometimes spaces are used), missing indents at times, random line breaks throughout your code, etc.

I put it through a beautifier, maybe I made some changes after that

- Your message event function is an absolute disaster. With all due respect, you need to seriously rework that. Using a switch and case as a command handler is a pretty daft way of command checking; look into using module.exports/export default function.

Bit harsh, could you explain a bit more what the problem is here?

In addition you are nested function declarations, which is one of the most dated JS practices I've ever seen. Don't do that unless you really have to. Your message event is pulling in unneeded params, you only need the msg object.

Yes I need to figure out how to pass variables to functions]

- From what I can tell you are hardlocking the prefix to ! which is a very common prefix by other bots and will cause a lot of issues in other servers. If you use a common prefix, make a command to change it.

Yes it's using ! - I'll review this, the servers I'm on don't have many bots

- Loading the base page you are doing an "if (err) throw err;". If the forums went down for even a second, this would crash your bot. Use proper error handling.
- COMMENT YOUR CODE. When it's got so many formatting issues, it's very difficult to analyse.

Both noted

- Reading this (and I could be wrong with this assumption), I am under the impression you have copied and pasted a lot of different snippets from various places.

Kind of, The discord bits have largely been copied/pasted and I have generally used a lot of snippets from various tutorials and stack overflow etc.
However the baseLoad, healthCompare and baseAlert functions, are my own.

- The end of your message event has a duplicate switch case that is testing against true. I can't even... what?

Well spotted

I would really suggest looking into async/await rather than simply relying on your setTimeout. Using setTimeout over and over (especially with your get requests and extremely long loops) can cause a lot of undesired behaviour and exceptions to be thrown. You are effectively going "yeah, this function will probably take about... err five seconds? I guess?".

async/await is something I have just started looking at and haven't gotten my head around yet.
A few of the timeouts are necessary in order to put longer messages in the right order on Discord

I'll end off by saying I personally find it extremely out of taste to request donations for a community project. I, like many of the others before me, have invested hundreds of hours into the development of this game and utilities for it and have never asked a penny for it. If this project took you months, it'd be a little more understandable, but something this small that is so evidently from an amateur just rubs me the wrong way.

I thought I would get a gripe on this and considered not including it. This project is a learning project for me, I'm not looking to make anything from the community for it. I've open sourced it so that I can have useful feedback, and so that if somebody else wants to use it in their projects or host it themselves they can.
However because some people that have had access to the earlier versions have said they would like to give something for it I decided to include the options. I'm not requesting a donation, go use it for free I don't mind, but if somebody wants to they can.

I've also seen a lot of developers quit over the years, including you Laz, because they've built all of these tools and content and don't feel appreciated or rewarded. Some community members have a mindset that developers/game masters/moderators are their servants and owe them something. Perhaps if we supported them instead of expecting them to provide everything to us then there'd be a happier developer base and more contribution. People happily pay for signatures/logos and even commodity deliveries, why not development?

---