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 #4961 (halo animations sometimes freeze) #5163

Merged
merged 1 commit into from Jun 28, 2022

Conversation

DisherProject
Copy link
Contributor

As this comment says, we are only redrawing haloes if anything else in the map is invalidating at least one hex.

halo_man_->unrender(invalidated_) actually invalidates the hexes covered by haloes which need to be updated, so i'm just moving the call outside the if-clause.

... when they're the only animated thing in vision

Fixes wesnoth#4961
@stevecotton
Copy link
Contributor

I'm not familiar with this code, but it looks like a minimal change on top of code that's already too complex.

However, it also looks like a change that increases the messiness of the code - moving a draws-to-screen call into a sets-invalidation-flags function. So overall I'm slightly against merging this.

@CelticMinstrel
Copy link
Member

For some reason, #4961 doesn't seem to be linked from this PR (though at least the commit is), so this comment endeavours to fix that.

@DisherProject
Copy link
Contributor Author

I'm not familiar with this code, but it looks like a minimal change on top of code that's already too complex.

Do you mean that it is likely to create some new issues?

However, it also looks like a change that increases the messiness of the code - moving a draws-to-screen call into a sets-invalidation-flags function. So overall I'm slightly against merging this

I could split the halo_manager::unrender method into two functions: the former would just invalidate the haloes, the latter would effectively unrender them. Then, i could call the invalidation here and the redraw here.

@stevecotton
Copy link
Contributor

Do you mean that it is likely to create some new issues?

I'm worried that it will, yes.

However, I'm unfamiliar with the code and not wanting to learn a new code section, I don't know what should be done with it.

@stevecotton stevecotton changed the title Fix #4961 Fix #4961 (halo animations sometimes freeze) Nov 4, 2020
@Wedge009 Wedge009 added the Graphics Issues that involve the graphics engine or assets. label Dec 12, 2021
@Pentarctagon
Copy link
Member

@mesilliac would this be made irrelevant by the rendering overhaul going on?

@mesilliac
Copy link
Contributor

Hmm. Eventually yes but not yet. From a quick look this could probably just be merged, as it's a small change. I'll take a look at it later.

@mesilliac mesilliac self-assigned this Jun 28, 2022
@mesilliac mesilliac removed the request for review from gfgtdf June 28, 2022 00:01
@mesilliac mesilliac merged commit 2d7e0a0 into wesnoth:master Jun 28, 2022
@mesilliac
Copy link
Contributor

Verified that it works and the change is okay. Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Graphics Issues that involve the graphics engine or assets.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants