-
Notifications
You must be signed in to change notification settings - Fork 14
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
Add transport unit #84
Comments
@djwhitt I had a lot of questions, so I'm consolidating them into this post.
|
|
Thanks for these answers. Follow ups: 3/4. I'm having a bit of trouble determining when a passenger unit can board a transport unit, if the transport unit is on a terrain the passenger unit cannot move to. It might be better for me to leave it unfixed for now and we can look more closely at the code together when I make a PR. And some new questions:
|
3/4. Ah, I hadn't fully thought through the implications of not being able to move to the terrain the transport is on. Pressing on sounds good and I'll keep thinking about it. Re viewing contained units, I was thinking about this a bit when you asked your previous set of questions and wondered what your plan was. 😄 I think a "View units contained" (or something similar) button + modal popup is the better option. I don't love either choice, but I haven't thought of anything better. I agree about the information displayed. Unit type and count are all that's needed. |
Cool, I was also thinking the button + modal popup would be better so I'll get that working soon. We can always replace it later if someone thinks of a better design. |
I'm returning to this with the Datascript knowledge I gained fixing #87 and I'm wondering if I should also use a relation instead of a regular value for the stored/contained units. |
Yes, units/units-contained should be relation. |
Thanks. |
I don't think a coordinating value is necessary. I'd just nest the passenger units under the transport when serializing the game state. Also, on a different note, I like the "passengers" term. I think :unit/passengers is better than :unit/units-contained as an attribute name. |
I've changed "units-contained" to "passengers" and fixed the |
I think splitting it into two actions is fine (move, then board adjacent transport). From a UI perspective, we may want to have a way to combine those (move + board in a single click), but don't worry about that until it's working. |
Cool. Should we also disembark passengers with a two step process because of the same terrain issues? |
Coming back to this, I'm remembering that the disembarking process was the biggest obstacle I was running into. Juggling unit selection gets messy. I'm thinking that the two step process (disembarking onto a bordering tile and then the former passenger being able to move from there) would be best. Thoughts? |
Yeah, I agree, two step disembarking makes sense. |
#67
The transport unit can carry personnel and armored units across bodies of water. See its definition in Elite Command: https://github.com/cvincent/elite-command/blob/c84a3afb3eb2aad3b88c9cc34a7c497caa156010/config/initializers/unit_definitions.rb#L1147
The text was updated successfully, but these errors were encountered: