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 visually overlapping sprites for wall spells #7208

Merged
merged 13 commits into from
Sep 2, 2024

Conversation

kphoenix137
Copy link
Collaborator

Fixes: #7205

Fixes the problem where wall spells create visible missiles that overlap in the center of the wall, resulting in double damage.

@qndel
Copy link
Member

qndel commented Jul 25, 2024

why touch all that whitespace stuff?

@kphoenix137
Copy link
Collaborator Author

why touch all that whitespace stuff?

what?

@qndel
Copy link
Member

qndel commented Jul 25, 2024

why touch all that whitespace stuff?

what?

all that stuff that was basically 2 duplicated blocks of code reusing variable names, you changed that to 2 sets of variables, why introduce unnecessary diff? Makes it harder to review ;)

@kphoenix137
Copy link
Collaborator Author

why touch all that whitespace stuff?

what?

all that stuff that was basically 2 duplicated blocks of code reusing variable names, you changed that to 2 sets of variables, why introduce unnecessary diff? Makes it harder to review ;)

Please provide suggested changed and I'll commit

Copy link
Contributor

@obligaron obligaron left a comment

Choose a reason for hiding this comment

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

Looks good to me, expect some naming suggestions. Thanks 🙂

Personally I would like to do more refactoring. For example introduce a ProcessWallControl that is used for both FireWall and LightningWall. Also I'm thinking about having startPositionLeft and startPositionRight and removing the { } that are only there for scoping. But all this is not directly related to the bugfix. 😁

Source/missiles.cpp Outdated Show resolved Hide resolved
Source/missiles.cpp Outdated Show resolved Hide resolved
Source/missiles.cpp Outdated Show resolved Hide resolved
Source/missiles.cpp Outdated Show resolved Hide resolved
Source/missiles.cpp Outdated Show resolved Hide resolved
Source/missiles.cpp Outdated Show resolved Hide resolved
Source/missiles.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@obligaron obligaron left a comment

Choose a reason for hiding this comment

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

thanks 🙂

@AJenbo AJenbo changed the title Fix Wall Spells Fix visually overlapping sprites for wall spells Sep 2, 2024
@AJenbo AJenbo merged commit 520bf5d into diasurgical:master Sep 2, 2024
22 checks passed
@AJenbo
Copy link
Member

AJenbo commented Sep 2, 2024

A word or two on what the issue is makes the log a bit more useful :)

@kphoenix137 kphoenix137 deleted the fix-wall-spells branch September 3, 2024 06:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Issue Report]: Wall spells produce twice as many visible missiles on the center tile
4 participants