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

Projectiles deal damage to breakable blocks #2357

Merged
merged 1 commit into from
May 17, 2015

Conversation

niamu
Copy link
Member

@niamu niamu commented May 14, 2015

One thing I noticed from watching some YouTubers play (thanks to @edisonout for providing a link to get me started down this path) is that one of the first things players do is craft throwing knives in the forest and then accidentally shoot them at the breakable blocks and realize that they don't do any damage.

After investigating the code, I realize that this wasn't intentional and have spent the last several hours working out a fix. Yes, it's stupid ugly. If you can come up with a cleaner fix, I'd love to see it.

This gets the job done and makes breakable blocks feel a bit better to interact with.

@CalebJohn
Copy link
Member

@niamu I think there is a better way to handle this, projectiles used to damage breakable blocks before the collision update so there should be a fix that doesn't use the new collision system.

@niamu
Copy link
Member Author

niamu commented May 14, 2015

Merged your commit @CalebJohn, but after trying this branch with only your changes not on top of mine, the collisions aren't as reliable.

I don't think this should be merged with both of our commits on this branch. I admit your change is cleaner, it just has reliability problems. It should only take 3 throwing knives to destroy the breakable block in the forest level. It takes several for me with your change alone.

@CalebJohn
Copy link
Member

@niamu I think this can be improved by shrinking the size of the projectile when used for hitting walls, I can try writing up a better fix tomorrow. I'm just not a huge fan of using the collision tile to work backwards to find the node.
I'll take another shot at this and if it doesn't work out I'll relent.

@niamu
Copy link
Member Author

niamu commented May 15, 2015

Totally get your objection to working backwards to finding the node that we are colliding with. It is very ugly. I like it only because it's proven to be reliable in my tests.

I'd welcome any cleaner fix you can come up with that matches reliability.

@niamu niamu force-pushed the breakable_blocks branch from 0899e7a to 987a4e5 Compare May 17, 2015 03:34
@niamu niamu force-pushed the breakable_blocks branch from 987a4e5 to a1ff576 Compare May 17, 2015 03:38
@niamu
Copy link
Member Author

niamu commented May 17, 2015

Thanks to @CalebJohn, this is now much cleaner and reliable now. LGTM.

@@ -222,7 +223,7 @@ function module.move_x(map, player, x, y, width, height, dx, dy)

if new_x <= tile_x and tile_x <= x then
if player.wall_pushback then
player:wall_pushback()
player:wall_pushback(tile)
Copy link
Member Author

Choose a reason for hiding this comment

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

The only reason we pass tile as an argument here is to return on the function before it runs. I think it's cleaner to move that condition up here before we even call that function. Thoughts?

@CalebJohn
Copy link
Member

@niamu We only ignore the function call in projectile.lua, this is because we want projectiles to ignore breakable blocks but anything else (e.g. the player) should not be ignoring the block so those specific functions wouldn't quit.

@niamu
Copy link
Member Author

niamu commented May 17, 2015

@CalebJohn, thanks for clarifying that. This is as clean as it gets then.

niamu added a commit that referenced this pull request May 17, 2015
Projectiles deal damage to breakable blocks
@niamu niamu merged commit a3eab58 into hawkthorne:master May 17, 2015
@niamu niamu deleted the breakable_blocks branch May 21, 2015 14:42
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.

2 participants