-
Notifications
You must be signed in to change notification settings - Fork 79
Fix wrong ressources stats behavior & Change Ressource Setting view #628
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
Changes from all commits
e89b298
1e2fb53
d1919ff
5787ad5
3ed5892
c2e1450
da8df8c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1459,7 +1459,6 @@ public function updateResourceProductionStats(bool $save_planet = true): void | |
| // 1. First time in order to ensure the energy production is up-to-date. | ||
| // 2. Second time for the mine production to be updated according to the up-to-date (actual) energy production. | ||
| $this->updateResourceProductionStatsInner($production_total, $energy_production_total, $energy_consumption_total); | ||
| $this->updateResourceProductionStatsInner($production_total, $energy_production_total, $energy_consumption_total); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great catch on identifying the duplicate method call! After revisiting this, it’s clear that this is one of the main causes of the discrepancy in resource production. The method itself directly appends the calculated production values to the planet object. So when called twice, it will append these values again, which isn’t correct. The reason the method is called twice was to respect the correct order of operations: first, energy production/consumption needs to be calculated, and then those totals are used to calculate mine production, as they rely on accurate energy data. However now that you have removed the second call, in regular gameplay it would now take two separate requests for everything to be properly calculated. This is why you saw the unit tests failing, as everything is done in a single request there. Adding back the second pass only in the unit tests is not ideal as the tests now have different logic compared to real player gameplay requests, though I understand your reasoning behind it for trying. It would likely be better to refactor
For naming, perhaps:
Then, for the third step, the logic for the fusion plant would rely on the output from step 2, so that could be handled as a separate part. If you have time to look at this further to improve it, feel free to do so. But if you don't I'm also happy to merge your current change as-is as it's already better than the previous implementation. The additional refactoring can then happen in a later PR. |
||
|
|
||
| if ($save_planet) { | ||
| $this->save(); | ||
|
|
@@ -1563,49 +1562,70 @@ public function getProductionForPositionBonuses(int $position): array | |
| */ | ||
| private function updateResourceProductionStatsInner(Resources $production_total, int|float $energy_production_total, int|float $energy_consumption_total, bool $save_planet = true): void | ||
| { | ||
| // Retrieve all energy production from building except for fusion plant. | ||
| foreach (ObjectService::getGameObjectsWithProduction() as $object) { | ||
| // Retrieve all game objects that have production values. | ||
| $production = $this->getObjectProduction($object->machine_name); | ||
| if ($object->machine_name != 'fusion_plant') { | ||
| $production = $this->getObjectProduction($object->machine_name); | ||
|
|
||
| if ($production->energy->get() > 0) { | ||
| $energy_production_total += $production->energy->get(); | ||
| } else { | ||
| // Convert negative production to positive consumption, same value. | ||
| $energy_consumption = abs($production->energy->get()); | ||
| if ($production->energy->get() > 0) { | ||
| $energy_production_total += $production->energy->get(); | ||
| } else { | ||
| // Convert negative production to positive consumption, same value. | ||
| $energy_consumption = abs($production->energy->get()); | ||
|
|
||
| $production_total->energy->add(new Resource($energy_consumption)); | ||
| $energy_consumption_total += $energy_consumption; | ||
| $production_total->energy->add(new Resource($energy_consumption)); | ||
| $energy_consumption_total += $energy_consumption; | ||
| } | ||
| } | ||
|
|
||
| // Combine values to one array, so we have the total production. | ||
| $production_total->add($production); | ||
| } | ||
|
|
||
| // Save the energy production and consumption values to the planet. | ||
| $this->planet->energy_used = (int)$energy_consumption_total; | ||
| $this->planet->energy_max = (int)$energy_production_total; | ||
|
|
||
| // After all production values are calculated, we need to calculate the actual fusion plant energy production. | ||
| // This is done by comparing deuterium consumption with deuterium production. If consumption is higher | ||
| // than production and there is no deuterium in storage, we need to set the energy production to 0. | ||
| foreach (ObjectService::getGameObjectsWithProduction() as $object) { | ||
| if ($object->machine_name === 'fusion_plant') { | ||
| // Retrieve the fusion plant production. | ||
| $production = $this->getObjectProduction($object->machine_name); | ||
|
|
||
| // If fusion plant deuterium production is negative (consumption) | ||
| // and there is no deuterium in storage, we need to set the energy production to 0. | ||
| $consumption_higher_than_production = abs($production->deuterium->get()) > $production_total->deuterium->get(); | ||
| if ($consumption_higher_than_production && $this->planet->deuterium == 0) { | ||
| // Remove energy production from previously calculated total. | ||
| $energy_production_total -= $production->energy->get(); | ||
| } | ||
| // If planet has deuterium in storage, the fusion plant can use the storage deuterium to produce energy | ||
| // In this case only the energy generated by the fusion plant can actualy be used to generate deuterium. | ||
| if ($this->planet->deuterium > 0) { | ||
| // Update the energy production of the planet with the fusion plant production. | ||
| $fusion_plant_production = $this->getObjectProduction('fusion_plant'); | ||
| $energy_production_total += $fusion_plant_production->energy->get(); | ||
| $this->planet->energy_max = (int)$energy_production_total; | ||
|
|
||
| // Calculate the deuterium production of the deuterium synthesizer WITH the fusion plant energy. | ||
| $deuterium_synthesizer_production = $this->getObjectProduction('deuterium_synthesizer'); | ||
| $production_total->add($fusion_plant_production); | ||
| $production_total->add($deuterium_synthesizer_production); | ||
| } | ||
| // If the planet has no deuterium in storage, the deuterium synthetizer CANNOT use the fusion plant energy to produce deuterium | ||
| else { | ||
| // Calculate the production of the deuterium synthesizer without updating the energy production of the planet. | ||
| $deuterium_synthesizer_production = $this->getObjectProduction('deuterium_synthesizer'); | ||
| $fusion_plant_production = $this->getObjectProduction('fusion_plant'); | ||
|
|
||
| // If the deuterium production is higher than the fusion plant consumption, we can use the fusion plant energy to produce other ressources. | ||
| $production_higher_than_consumption = abs($fusion_plant_production->deuterium->get()) < $deuterium_synthesizer_production->deuterium->get(); | ||
| if ($production_higher_than_consumption) { | ||
| $energy_production_total += $fusion_plant_production->energy->get(); | ||
| $this->planet->energy_max = (int)$energy_production_total; | ||
| $production_total->add($fusion_plant_production); | ||
| $production_total->add($deuterium_synthesizer_production); | ||
| } | ||
| // As a professional ingeeneer, we will suppose that the fusion plant will switch to Safety program | ||
| // And will stop producing energy if not enough deuterium is available. | ||
| // To ensure that the working technicians stay safe the fusion plant will still consume the deuterium to avoid explosions. | ||
| // TODO : Ask to an Ogame professional Engineer Doctor what the fusion plant is supposed to do. | ||
| } | ||
|
|
||
| // Calculate the metal & crystal production normaly. | ||
| $production_total->add($this->getObjectProduction('metal_mine')); | ||
| $production_total->add($this->getObjectProduction('crystal_mine')); | ||
|
|
||
|
Comment on lines
+1621
to
+1624
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of hardcoding production of metal/crystal/deuterium by looking directly at the buildings This is important because it's possible that in the future other buildings will be added that can produce metal/crystal/deut, and this method should be generic enough that it automatically picks them up without needing to change the code. So I advise to look at the original implementation before your changes, and use that as a reference point for how to do the calculation.
|
||
| // Write values to planet. | ||
| $this->planet->metal_production = (int)$production_total->metal->get(); | ||
| $this->planet->crystal_production = (int)$production_total->crystal->get(); | ||
| $this->planet->deuterium_production = (int)$production_total->deuterium->get(); | ||
|
|
||
| $this->planet->energy_used = (int)$energy_consumption_total; | ||
| $this->planet->energy_max = (int)$energy_production_total; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -1643,17 +1663,23 @@ public function getObjectProduction(string $machine_name, int|null $object_level | |
| $resource_production_factor = $this->getResourceProductionFactor(); | ||
| } | ||
|
|
||
| $building_percentage = !$force_factor ? $this->getBuildingPercent($machine_name) : 100; | ||
| $building_percentage = !$force_factor ? $this->getBuildingPercent($machine_name) : 10; | ||
| $planet_temperature = $this->getPlanetTempAvg(); | ||
| $planet_max_temperature = $this->getPlanetTempMax(); | ||
| $energy_technology_level = 0; // Implement energy technology level getter. | ||
| $energy_technology_level = $this->player->getResearchLevel('energy_technology'); | ||
| $universe_resource_multiplier = $this->settingsService->economySpeed(); | ||
|
|
||
| $production = new Resources(0, 0, 0, 0); | ||
|
|
||
| $production->metal->set((eval($gameObject->production->metal) * $universe_resource_multiplier) * ($resource_production_factor / 100)); | ||
| $production->crystal->set((eval($gameObject->production->crystal) * $universe_resource_multiplier) * ($resource_production_factor / 100)); | ||
| $production->deuterium->set((eval($gameObject->production->deuterium) * $universe_resource_multiplier) * ($resource_production_factor / 100)); | ||
| // Fusion plant does not require energy tu consume deuterium so it should not be impacted by resource_production_factor | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typo: |
||
| if ($machine_name === 'fusion_plant') { | ||
| $production->deuterium->set((eval($gameObject->production->deuterium) * $universe_resource_multiplier)); | ||
| // Fusion Reactor is powered by deuterium which is not impacted by universe resource multiplier | ||
| } else { | ||
| $production->deuterium->set((eval($gameObject->production->deuterium) * $universe_resource_multiplier) * ($resource_production_factor / 100)); | ||
| } | ||
| $production->energy->set((eval($gameObject->production->energy))); // Energy is not affected by production factor or universe economy speed. | ||
|
|
||
| // Round down for energy. | ||
|
|
@@ -1673,16 +1699,16 @@ public function getObjectProduction(string $machine_name, int|null $object_level | |
| * This percentage indicates how efficient the resource buildings (mines) | ||
| * are functioning. | ||
| * | ||
| * @return int | ||
| * @return float | ||
| * The production factor expressed as a percentage (min 0, max 100). | ||
| */ | ||
| public function getResourceProductionFactor(): int | ||
| public function getResourceProductionFactor(): float | ||
| { | ||
| if (empty($this->energyProduction()->get()) || empty($this->energyConsumption()->get())) { | ||
| return 0; | ||
| } | ||
|
|
||
| $production_factor = $this->energyConsumption()->get() ? floor($this->energyProduction()->get() / $this->energyConsumption()->get() * 100) : 0; | ||
| $production_factor = $this->energyConsumption()->get() ? $this->energyProduction()->get() / $this->energyConsumption()->get() * 100 : 0; | ||
|
|
||
| // Force min 0, max 100. | ||
| if ($production_factor > 100) { | ||
|
|
@@ -1691,7 +1717,7 @@ public function getResourceProductionFactor(): int | |
| $production_factor = 0; | ||
| } | ||
|
|
||
| return (int)$production_factor; | ||
| return $production_factor; | ||
| } | ||
|
|
||
| /** | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.