search

spAnser / discord-coc-war-announcer

A node.js discord bot written to monitor the Clash of Clans API and announce war attacks to a discord channel. spAnser#2031
MIT License
29 stars 24 forks source link

CWL & Emoji update, change README, add @everyone tag on missing attacks. #25

Closed curtiola closed 5 years ago

curtiola commented 5 years ago

Added emoji images for Ice Golem, Stone Slammer, Bat Spell, BH9 & Hog Glider. Added additional emoji information to README.md Left their ID #s blank in index.js since I am not hosting the bot publicly. Added '@everyone' before the missing attacks embed, as suggested in issue #3.

spAnser commented 5 years ago

I think you changed the TH emoji IDs in this one?

I will most likely merge this weekend though, just been pretty busy.

curtiola commented 5 years ago

I did change them by mistake at first, then yesterday I changed the ID #s back to the original ones you used. (Still leaving the new emoji ID #s blank) Since first opening the PR I also added the changes from the Jonathan-Law fork that updated stuff for CWL. He overhauled index.js, clans.js & added separate parseCurrentWar.js & parseCurrentLeague.js

spAnser commented 5 years ago

Sounds good then.

curtiola commented 5 years ago

!unannounce was broken in the Jonathan-Law index.js changes, fixed now.

Everything else seems fine.

curtiola commented 5 years ago

For CWL wars, it thinks everyone has 2 attacks, so that's not great. Not sure what to do about it right now, looking into it.

spAnser commented 5 years ago

No problem.

curtiola commented 5 years ago

Will reopen when I've tested this more.

curtiola commented 5 years ago

Lil status update; CWL can designate our clan as the opponent, causing showmissing to announce the enemy missing attacks & using the wrong clan tag for a lot of the things. Unsure exactly what to do right now, need to add a clan tag match check or something, slowly workin on it, but I am the big dumb lol.. I'll get 'er figured eventually but if you have any tips hit me with them please.

spAnser commented 5 years ago

Feel free to re open the pull request and I can merge it and look through stuff maybe figure it out. It is only on the develop branch anyways.

spAnser commented 5 years ago

I actually have some code in my local version for claiming accounts with the API token I need to merge in anyways. I'm not sure if they ever made that endpoint public yet so I was waiting for that before pushing.

curtiola commented 5 years ago

Oh ok cool, I'll check that out when/if you merge it. Also, unsure if storing the CWL true/false string in ./node-persist/clan-storage was the best way to do it, but that's what I did.