Ikabot-Collective / ikabot

A Python-based bot designed for automating tasks in the game Ikariam
https://discord.com/invite/3hyxPRj
MIT License
97 stars 65 forks source link

Custom web server port #268

Closed susikaman closed 3 months ago

susikaman commented 3 months ago

Option for selecting a custom port for the web server to use, really useful for consistency.

susikaman commented 3 months ago

This is definitely a small improvement, but an improvement nonetheless. Generally I don't see why people would want to chose the port manually, unless they are trying to serve their ikariam account over some external service like Cloudflare (which only allows firewall rules for specific port numbers). I think maybe adding the port into the config.py file or asking for port input only when some flag is set to true in config.py would be a better option, so as to not confuse people who are less tech-savvy and may not know what a port even is.

Regardless, I still think this is a good change, so I'll leave it up to you to decide whether or not to add the config.py flag change.

The point is to have consistency for the web server feature to make things easier. Isn't it ideal to keep the address (127.0.0.1:xxxxx) always the same, rather than having to re-check the correct address/port constantly?

But yeah, a config option would work just fine as well. As-is the PR just prompts for a port, but continues with the original concept if the field is passed empty or 0. So not much of a difference imo, just one extra enter haha

ikagod commented 3 months ago

The point is to have consistency for the web server feature to make things easier. Isn't it ideal to keep the address (127.0.0.1:xxxxx) always the same, rather than having to re-check the correct address/port constantly?

The port number is derived from the user's email and the account's server id (eg. s301-en). This is done to ensure that the same account always gets assigned the same port number. Maybe the user is running ikabot on multiple accounts, and wants to use the web server feature on all of them. This way, they will all be assigned a (semi) unique and CONSISTENT port number, The chance two accounts get the same port number is one is two thousand, which I think is sufficient unless you are using ikabot on thousands of accounts. This PR is fixing an issue that doesn't exist, but as I've said previously, letting the user chose a custom port number could have it's advantages in solving other issues, like the use of external services that only allow certain port numbers for communication, but in that case it should be a little harder to chose the port manually, so as to not inconvenience most users who do not need to set the port manually.

susikaman commented 3 months ago

The point is to have consistency for the web server feature to make things easier. Isn't it ideal to keep the address (127.0.0.1:xxxxx) always the same, rather than having to re-check the correct address/port constantly?

The port number is derived from the user's email and the account's server id (eg. s301-en). This is done to ensure that the same account always gets assigned the same port number. Maybe the user is running ikabot on multiple accounts, and wants to use the web server feature on all of them. This way, they will all be assigned a (semi) unique and CONSISTENT port number, The chance two accounts get the same port number is one is two thousand, which I think is sufficient unless you are using ikabot on thousands of accounts. This PR is fixing an issue that doesn't exist, but as I've said previously, letting the user chose a custom port number could have it's advantages in solving other issues, like the use of external services that only allow certain port numbers for communication, but in that case it should be a little harder to chose the port manually, so as to not inconvenience most users who do not need to set the port manually.

I literally just debunked this by running 5 accounts simultaneously, and starting the web server task by predetermined input. Each instance launches in 20 second interval, so they're not even launching at the same time. Regardless, 4/5 of the instances are now using the same port, while only the last instance has a different port by 1 digit.

So yeah, I believe this PR is a strong improvement.

ikagod commented 3 months ago

I literally just debunked this by running 5 accounts simultaneously, and starting the web server task by predetermined input. Each instance launches in 20 second interval, so they're not even launching at the same time. Regardless, 4/5 of the instances are now using the same port, while only the last instance has a different port by 1 digit.

Let's not be so hasty, we have time to discuss this. If what you say is true, then that indicates that there must be a bug somewhere in the code. I can not test this myself because I do not use multiple accounts and never have.

Are all the accounts under the same email (same lobby)? Are they all on the same world server? If yes, then there is the issue. We can fix this by including the world server player name in the randomness that derives the port number, and then all your accounts will consistently be using randomly assigned port numbers.

I think fixing this bug might be the answer, and not clogging up the UI by asking the person to input their desired port number on every run of the ikabot web server.

Please let me know if all your accounts use the same email and are on the same world server.

susikaman commented 3 months ago

I literally just debunked this by running 5 accounts simultaneously, and starting the web server task by predetermined input. Each instance launches in 20 second interval, so they're not even launching at the same time. Regardless, 4/5 of the instances are now using the same port, while only the last instance has a different port by 1 digit.

Let's not be so hasty, we have time to discuss this. If what you say is true, then that indicates that there must be a bug somewhere in the code. I can not test this myself because I do not use multiple accounts and never have.

Are all the accounts under the same email (same lobby)? Are they all on the same world server? If yes, then there is the issue. We can fix this by including the world server player name in the randomness that derives the port number, and then all your accounts will consistently be using randomly assigned port numbers.

I think fixing this bug might be the answer, and not clogging up the UI by asking the person to input their desired port number on every run of the ikabot web server.

Please let me know if all your accounts use the same email and are on the same world server.

They are indeed on the same lobby and same server. Perhaps the player name would make the most difference. But still I don't see why there couldn't be an option to select the port by yourself, for whatever reason. For myself, the reason is simplicity and easy-to-remember functionality. It's way easier to do 0.0.0.0:44441, 0.0.0.0:44442 etc than trying to remember all randomized port numbers.

ikagod commented 3 months ago

Please test again with the new commit. I still think hiding this port prompt behind some askForWebServerPort flag in config.py would be a better option.

susikaman commented 3 months ago

Please test again with the new commit. I still think hiding this port prompt behind some askForWebServerPort flag in config.py would be a better option.

Tested and works perfectly fine now, ports are vastly different for accounts in the same worlds. But I still insist that it's very convenient for those who want to use an easy-to-remember port.

ikagod commented 3 months ago

Tested and works perfectly fine now, ports are vastly different for accounts in the same worlds. But I still insist that it's very convenient for those who want to use an easy-to-remember port.

for those who want to use an easy-to-remember port.

As I've said, the prompt can be skipped unless some flag is set to True in config.py, so as to not clutter the UI with unnecessary prompts that most people won't care about or ever use, that is my final OPINION. But, as I've said before, I will leave it completely up to you to decide whether this prompt will be displayed to everyone or only the users who know about this flag and have set it to True.

I will fully accept and stand by whichever decision you make.

susikaman commented 3 months ago

Tested and works perfectly fine now, ports are vastly different for accounts in the same worlds. But I still insist that it's very convenient for those who want to use an easy-to-remember port.

for those who want to use an easy-to-remember port.

As I've said, the prompt can be skipped unless some flag is set to True in config.py, so as to not clutter the UI with unnecessary prompts that most people won't care about or ever use, that is my final OPINION. But, as I've said before, I will leave it completely up to you to decide whether this prompt will be displayed to everyone or only the users who know about this flag and have set it to True.

I will fully accept and stand by whichever decision you make.

I added a config flag for the custom port functionality. I also thought of having way more of these, but use the config values as a default for dynamic settings. See, once version 8.0.0 is out, we can use dynamic settings from the userfile instead of these hardcoded flags within the config file which would be a lot more convenient. And I believe there are a lot more of these additional prompts and options which some users don't care for and some do. We could very easily add all of these into the config file as default values for the userfile to fetch from upon creation, and users could easily modify these settings directly from Ikabot and save their personalized settings into their userfile. Tell me your thoughts on this idea.

ikagod commented 3 months ago

I added a config flag for the custom port functionality. I also thought of having way more of these, but use the config values as a default for dynamic settings. See, once version 8.0.0 is out, we can use dynamic settings from the userfile instead of these hardcoded flags within the config file which would be a lot more convenient. And I believe there are a lot more of these additional prompts and options which some users don't care for and some do. We could very easily add all of these into the config file as default values for the userfile to fetch from upon creation, and users could easily modify these settings directly from Ikabot and save their personalized settings into their userfile. Tell me your thoughts on this idea.

Sounds like a great idea. Merge this PR whenever you thinks it's ready.

susikaman commented 3 months ago

I added a config flag for the custom port functionality. I also thought of having way more of these, but use the config values as a default for dynamic settings. See, once version 8.0.0 is out, we can use dynamic settings from the userfile instead of these hardcoded flags within the config file which would be a lot more convenient. And I believe there are a lot more of these additional prompts and options which some users don't care for and some do. We could very easily add all of these into the config file as default values for the userfile to fetch from upon creation, and users could easily modify these settings directly from Ikabot and save their personalized settings into their userfile. Tell me your thoughts on this idea.

Sounds like a great idea. Merge this PR whenever you thinks it's ready.

I don't have write access to the main repo, but regardless it's good now. Whoever can, may merge this whenever possible.