Feature - extend building queue requirements

[rautamik]

Related to: #299

Changes:

• Check object prior levels when object requirements are checked. Eg. check that robot_factory 1 exists when robot_factory 2 requirements are checked. Remove old check from build and research queue services.
• Accept build and research queue items when object requirements are checked. Eg. when shipyard requirements are checked accept required robot_factory 1 and 2 from build queue.
• Check that requirements are still met when item is taken from build or research queue for processing, if requirements are not met the item building is canceled.
• Check all items in the building and research queue when building or research are cancelled and remove all objects that are not passing requirements check,
• Add tests for building queue and research queue for checking if item is in queue and canceling item when requirements are not met.

Todo:

• Fix tests broken after rebase
• Fix starting research/building when requirements are in queue/progress and not yet ready

[rautamik]

I noticed one issue with this PR. If research lab is started to be built, then it's possible to start research even the research lab is not ready. I think this is because in research queue start function requirements check building queue items are accepted when research requirements are checked. I guess that building/research queue items should not be accepted when starting to build new object, only existing items should be accepted. I will look into this.

[rautamik]

Another thing is that I didn't run the unit tests after rebase and some cases seems to fail now. I will look into this.

[lanedirt]

Hi @rautamik,

Thanks for your investigation and work on this so far, sounds promising!

I'll wait until you give the green light that the changes are working stable, afterwards I'll gladly review the code and try testing it on my part with various use cases as well.

Thanks again for your contributions, greatly appreciated 🚀 !

[rautamik]

Hi @lanedirt,

Is the goal to make crosschecks between building and research queues, for example accept queueing research if required level of research lab is in building queue? If the goal is to do crosschecks then there are following use cases: research lab level 1 is currently building and user is able to add computer technology to research queue as requirements are met, but the item is immediately canceled as the requirements are not ready when researching is started.

Any other use cases to cover than the basic use case (robotics factories + shipyard)?

[lanedirt]

Hi @rautamik,

Thanks for your time and progress on this feature, it's looking great!

I thought a little bit about your question, and I think, based on my current understanding of the official game, that we do not need to do crosschecks between buildings and technology.

A concrete example from the top of my head:

• Technology should only be allowed to be queued if the research lab and required level are fully constructed at that moment. For example, if a technology needs a level 5 research lab, it should only be able to be queued when level 5 research lab is actually active at that exact moment.

Another thing that might also be relevant to this feature:

• Once a research lab exists (level > 0), both lab upgrades and new research should be able to be added to the queue at the same time. However, active research and an active lab upgrade shouldn’t be able to happen at the same time. This logic is not implemented yet but is described in a separate issue: [BUG] Disallow research when a research lab is upgrading on any planet #181.

The other example you mention of the robotics factory and shipyard is however valid because those two belong to the same building queue.

Does this answer your question? Disclaimer: I'm relying on my own understanding of the official game. I might not be fully correct on some details. But of course when the feature is ready for testing I'll be happy to help out with testing and verifying the logic and to compare it to the official game.

Thanks again for your work on this, I'm really happy about your contributions and progress 😄 !

[rautamik]

Hi @lanedirt,

Thanks for the feedback! I have made some adjustments based on the feedback.

If you don't mind I would keep the #181 issue separate from this PR even though it's closely related. I can look that case next after this PR is ready if you want.

I think that everything should be in place now regarding issue #299. Unless I have missed something. You could try it out when you have time and let me know if there are any issues or it's not working as it should be, thanks.

If everything is working fine then I'm looking forward for code review and will be glad to make any needed adjustments.

It has been a pleasure to work in this project.

[lanedirt]

Hi @rautamik,

I just tested and reviewed the changes and it's looking good! I do have a few minor comments regarding namespaces and typos, see my comments inline. And good idea to keep the #181 issue separate, I agree.

--

Other than the small inline comments I did come across a new game logic bug which I didn't think of before.

Example:

1. Queue robotics factory level 1 + 2 in build queue.
2. Queue shipyard level 1 in build queue.
3. Now when I navigate to the shipyard page the Solar Satellite object can already be built while the Shipyard is not yet built (it's still in the queue).

In this case for being able to build units, the shipyard level requirement should actually be finished (built) and should not look what is in the queue.

I also noticed the same thing applies to technology/research. If I queue energy technology lvl 1 + combustion drive lvl 1, the light fighter becomes immediately buildable. In this case for unit requirements it should also only look for what is actually researched/built and not look in the queue.

Perhaps this is a good use case for adding an additional test to ensure this game logic is consistently tested with every change.

Sidenote: I think the official game logic seems relatively simple from the outside, but I have to say that as we dig deeper into implementing all the finer details, it's clear there are quite a few edge cases and potential loopholes to consider. But that’s also what makes it interesting IMO! 😄

Could you have a look at these comments and suggestions? Thanks for your amazing work so far 🚀 !

[rautamik]

Hi @lanedirt,

Thanks for the feedback! Good comments and suggestions.

I will fix the unit queue issues and add more test cases. I'm new to the game and missed totally the unit queue, sorry for that.

Regarding unit/feature tests, you are totally right and I will see if I changes those back as unit tests or if not then I will move those properly as feature tests.

I will look also into other issues and suggestions that you raised, thank!

[rautamik]

Hi @lanedirt,

I have addressed all the issues you raised and made the feature test coverage better. Please check the PR when you have time, thanks.

[lanedirt]

Hi @rautamik,

Thanks for making the changes! Just tested it and works flawlessly on my end. 👍 Awesome work. Will merge the PR!