Bugfix/lab upgrading disallow researching

rautamik commented 1 week ago

Bugfix to https://github.com/lanedirt/OGameX/issues/181

Changes:

Refactor object building requirements check. Split object requirements met check to different functions: base, with previous levels and with items in queues.
Disallow research when Research Lab is upgrading.

TODO;

[x] check Research Lab upgrading from all planets, not only from current planet
[x] disallow Research Lab upgrading when research is in progress.
[ ] disable all Research Lab upgrade buttons when research is in progess
[ ] fix multiplanet research cancellation bug

rautamik commented 5 days ago

Hi @lanedirt,

Please check this object requirement check refactoring and fix to research lab upgrading PR when you have time.

Both cases, researching and research lab upgrading, should be covered and I have also included feature tests for both cases.

lanedirt commented 5 days ago

Hi @rautamik,

Thanks for your work on this! I just tested it on my local environment and looks very good overall, but I did notice two small things, see below. I also glanced over the code and it looks good to me. 👍 Also major thanks for taking the time to add the feature tests, that will surely help out with future changes or refactoring if needed!

Here are the two things I noticed:

1) Resarch lab buttons are not all disabled When research is being carried out, the research lab shows the "Research is being carried out" which is good. However the upgrade button is still active for the "quick upgrade" and the normal upgrade buttons. I would expect these two buttons to be disabled as well:

2) Trying to build research lab results in research itself getting canceled An additional strange thing I encountered after playing with it a bit is the following:

Start research on planet A
Switch to planet B
Attempt to upgrade the research lab. The research lab itself is grayed out but because the "build" button still being pressable (related to first issue mentioned above). When I press the build button nothing happens, i.e. the research lab doesn't get added to the queue which is good. However what I noticed is that after this action the actual research that was in progress is also canceled. I would expect the research to stay in the queue and not be affected. Note: this only happened to me when I try to build the research lab on a different planet. If I do both things on the same planet this issue doesn't happen.

rautamik commented 3 days ago

Hi @lanedirt,

Thanks for testing and providing feedback.

I should have noticed that Improve button needs to be disabled, I have now committed changes to disable it. I didn't even know about the quick upgrade button, thanks for pointing out this. I have also fixed this issue.

Regarding the second issue, canceling the research, I was not able to reproduce it when the Improve button was still visible. Like you said, nothing happens to the Research Lab when it's tried to upgrade during the research, and that is because there is sanity check when build queue item is started. I was trying it by researching Energy Technology level 5 in planet A and trying to upgrade Research Lab level 3 to level 4 in planet B. Research should get cancelled only if requirements are missing so I wonder if there is something like it happens when specific level of Research Lab is being cancelled or something like that. In any case it should be possible to happen when players cannot start upgrading, when all buttons are properly disabled. I wonder if we could skip this issue, what you think?

lanedirt commented 3 days ago

Hi @lanedirt,

Thanks for testing and providing feedback.

I should have noticed that Improve button needs to be disabled, I have now committed changes to disable it. I didn't even know about the quick upgrade button, thanks for pointing out this. I have also fixed this issue.

Regarding the second issue, canceling the research, I was not able to reproduce it when the Improve button was still visible. Like you said, nothing happens to the Research Lab when it's tried to upgrade during the research, and that is because there is sanity check when build queue item is started. I was trying it by researching Energy Technology level 5 in planet A and trying to upgrade Research Lab level 3 to level 4 in planet B. Research should get cancelled only if requirements are missing so I wonder if there is something like it happens when specific level of Research Lab is being cancelled or something like that. In any case it should be possible to happen when players cannot start upgrading, when all buttons are properly disabled. I wonder if we could skip this issue, what you think?

Hi @rautamik,

Thanks for the fixes! I'll try to test the new version again later today.

Regarding the issue: ah yes I guess the level of the research lab might be the cause then. My planet B was empty, so when I tried to upgrade the research lab it was from level 0 to level 1. And after canceling the upgrade, the research lab on that planet is too low for the research in the queue (because level 0) so it cancels the research itself too.

I agree that if the upgrade button now disabled fixes this problem we can leave it at this. I'll double check this on my end in my tests later. 👍

Thanks again for your work 😄 !

lanedirt / OGameX

Bugfix/lab upgrading disallow researching #443