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

[TESTED]Added Auto Contract button next to ask for easier & changed emoji for easier button #5573

Open
wants to merge 32 commits into
base: master
Choose a base branch
from

Conversation

I-am-TURBO
Copy link
Contributor

@I-am-TURBO I-am-TURBO commented Jan 12, 2024

Description:

help it stand out from the auto farming contract button
Screenshot_20240109_201433_Discord

Changes:

  • added Auto farming Contract button to go next to Ask for Easier
  • Changed Emoji of Ask for easier button

Other checks:

  • I have tested all my changes thoroughly.
screenshot of change ![SmartSelect_20240427_061146_Discord.png](https://github.com/oldschoolgg/oldschoolbot/assets/133211494/ca3a2cf8-1bf9-4b81-9df3-e367462e4cc3)

This way it stands out from the auto farming contract and auto Farm buttons
This way we don't have to scroll back up in order to press the farming contract button again and prevents people from accidentally asking for an easier contract
Copy link
Contributor

@LuxrayElite LuxrayElite left a comment

Choose a reason for hiding this comment

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

Currently you've only changed the emoji, this is an empty commit.

I saw in the closed pr that you do have the code for the suggested button change.

Unsure if a dice is a suitable alternative emoji (but not up to me)

@I-am-TURBO
Copy link
Contributor Author

I-am-TURBO commented Jan 12, 2024

Currently you've only changed the emoji, this is an empty commit.

I saw in the closed pr that you do have the code for the suggested button change.

Unsure if a dice is a suitable alternative emoji (but not up to me)

so for the life of me I'm not sure what happened to the file I had saved that had all the error fixing already done so I just went through it all again. but it is now back to where it was before with circular tests failing, which @themrrobert said is something I can't do on my own.

and @themrrobert gave me the idea of changing the one emoji and when I asked for suggestions he came up with the dice.

@nwjgit
Copy link
Contributor

nwjgit commented Jan 12, 2024

Your circular dependency issue can be fixed by adding:

const autoContractButton = new ButtonBuilder()
.setCustomId('AUTO_FARMING_CONTRACT')
.setLabel('Auto Farming Contract')
.setStyle(ButtonStyle.Secondary)
.setEmoji('977410792754413668');

before the farmingContractCommand function.

tested here: https://discord.com/channels/940758552425955348/1162793245194072074/1195448092665847831

Copy link
Contributor Author

@I-am-TURBO I-am-TURBO left a comment

Choose a reason for hiding this comment

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

thank you for this. I see what you did and makes so much more sense now.

@gc
Copy link
Collaborator

gc commented Feb 2, 2024

Do you have a test bot you're testing this on?

@I-am-TURBO
Copy link
Contributor Author

Do you have a test bot you're testing this on?

unfortunately not. cyr also worked with me on the coding for this one too.

@nwjgit
Copy link
Contributor

nwjgit commented Feb 2, 2024

Do you have a test bot you're testing this on?

I tested it here: https://discord.com/channels/940758552425955348/1162793245194072074/1195448092665847831 after i helped fix the circular issue.

Discord_X6qa46wWGP

@I-am-TURBO I-am-TURBO changed the title Added Auto Contract button next to ask for easier & changed emoji for easier button [TESTED]Added Auto Contract button next to ask for easier & changed emoji for easier button Apr 27, 2024
Copy link
Contributor Author

@I-am-TURBO I-am-TURBO left a comment

Choose a reason for hiding this comment

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

Important

@gc @themrrobert any chance this PR can get looked over to be merged?

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

4 participants