-
Notifications
You must be signed in to change notification settings - Fork 58
Add basic bombard by land units #928
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
Merged
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
b22a72d
Init bombard (WIP)
ajhalme 93eb576
Bombard grid via direct draw, basic UI interactions, enine msg, save …
ajhalme 7fc12de
unit-bombard Godot keymap
ajhalme d692cec
Bombard target tile highlight
ajhalme 33ef574
Add rateOfFire
ajhalme 7f86aeb
Adjsut unitProtos with bombard details
ajhalme c2371b7
Disable animation for CreateGame first turn
ajhalme f00f792
effect animation, z-indexing, bombard rate of fire and notice
ajhalme 8b0171f
Tiles within bombard range
ajhalme c3b375f
Tile square improvements
ajhalme 84fd602
Bombard refinement: basic city and terrain bombing, more bombard targ…
ajhalme a088bdb
Merge Development
ajhalme fa310d5
Test file change
ajhalme d3c28ea
Refine bombard rules
ajhalme 6f02d3c
Remove load-time animation issue workaround
ajhalme a0aba9f
RemoveCitizen revisited
ajhalme 8b744f6
remove null conditional
ajhalme fc333a6
Clean up Terrain Improvement life cycle
ajhalme 90fafc9
Sprinkled some "TODO: Make Configurable" comments
ajhalme 8ffe4a6
Merge Development
ajhalme 64958cc
Add Cursor.png
ajhalme 7a2d6fc
Patch Assets
ajhalme File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Submodule Assets
updated
3 files
| + − | Art/Civilopedia/icons/units/unit_large.png | |
| + − | Art/Civilopedia/icons/units/unit_small.png | |
| + − | Art/Cursor.png |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a comment on this particular PR, but since you had some friction with it now, does this whole system look ok to you (AnimationManager, C7Animation, AnimationTracker, etc)?
Let me know if you have any comments, I am trying to refactor this on the side, make it more versatile and extendible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current effect animation feels very glued-on on top of the unit animation scheme. Rethinking/refactoring the animation system as a whole makes sense.
The bomber plane bombardment animation might be a good starting point as it's a complex sequence. If you can support that, then everything else is easy.
I kinda like the approach of having the animation scheduling and progress-dependent rendering be separate parts. It's a bit much initially, but I can see how it can be really flexible. Maybe we need a central scheduling scheme and then a few different renderers. Could have simpler tile-based ones for unit and effect, and then a more complex renderer that can handle moving the render target on screen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to move away from tile-based animations altogether, and move to X/Y (screen?) coordinate animations. Tile based anything seems to be very restrictive as it is right now.
Stuff that are not doable with the current setup: