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

Fix incorrect resource placements on certain maps #6395

Merged
merged 4 commits into from
Dec 19, 2022

Conversation

ihhub
Copy link
Owner

@ihhub ihhub commented Dec 18, 2022

close #6393

This change must be thoughtfully tested on many maps to make sure that we don't break anything. The test scope should be done on maps with "bad" resources as well as any normal resources.

@ihhub ihhub added bug Something doesn't work logic Things related to game logic labels Dec 18, 2022
@ihhub ihhub added this to the 1.0 milestone Dec 18, 2022
@ihhub ihhub self-assigned this Dec 18, 2022
@ihhub ihhub requested review from idshibanov and Branikolog and removed request for idshibanov December 18, 2022 14:04
Copy link
Collaborator

@Branikolog Branikolog left a comment

Choose a reason for hiding this comment

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

A miracle is here! :)
Resource is weirdly looking, being rendered over mountains, If it's not possible, to render it under mountains, like it was in the OG, I cannot imagine any better solution here.

@ihhub
Copy link
Owner Author

ihhub commented Dec 18, 2022

Hi @Branikolog , please test this issue on normal resources being generate on multiple maps as well as the underlying logic for placing them was modified.

@ihhub
Copy link
Owner Author

ihhub commented Dec 18, 2022

@Branikolog , I also added an error log in case if incorrect resource type would appear on any map. Try to load dozens of maps to see if you get the error message.

@Districh-ru
Copy link
Collaborator

Districh-ru commented Dec 18, 2022

Hi, @ihhub, @Branikolog, a couple of months ago I found a bug in "Voyage Home" expansion company, map 2. It has an unknown mine. But OG also has the same bug and it does not lead to a game crush, so I forgot about it :)

изображение

Save file: vh2_bad_mine.zip

PS I don't ask to fix in in this PR, just remembered. :)

@sonarcloud
Copy link

sonarcloud bot commented Dec 18, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@oleg-derevenetz
Copy link
Collaborator

@Districh-ru see #5537 :)

@Districh-ru
Copy link
Collaborator

@Districh-ru see #5537 :)

Thanks, I'll add my post with the save file to this issue.

Copy link
Collaborator

@zenseii zenseii left a comment

Choose a reason for hiding this comment

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

I've loaded up every single scenario map that comes with the GOG release and every single campaign scenario with DBG_GAME turned on, and searched for the string "contains unknown resource type" in the log file and nothing turned up.

@Districh-ru
Copy link
Collaborator

@ihhub, I also have checked all maps I have (it's a standard set from Buka CD) and did not find any unknown resource type.
Also I checked on what maps works your fix: maps 'peasants.mx2' and 'whoam.mp2' has 1 resource under an object. And map 'lostreli.mp2' has 5.

Copy link
Collaborator

@oleg-derevenetz oleg-derevenetz left a comment

Choose a reason for hiding this comment

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

LGTM. Could you please apply the IWYU suggestion? Also do you want to leave the ERROR_LOG? I used the DEBUG_LOG with DBG_GAME and DBG_WARN in similar cases.

@ihhub
Copy link
Owner Author

ihhub commented Dec 18, 2022

LGTM. Could you please apply the IWYU suggestion? Also do you want to leave the ERROR_LOG? I used the DEBUG_LOG with DBG_GAME and DBG_WARN in similar cases.

Oh, I missed IWYU suggestion again :( I changed logging to DEBUG_LOG.

@sonarcloud
Copy link

sonarcloud bot commented Dec 18, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@ihhub ihhub merged commit fb476f3 into master Dec 19, 2022
@ihhub ihhub deleted the invalid_resource_placement branch December 19, 2022 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something doesn't work logic Things related to game logic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assertion on resource collecting
5 participants