Zetawar / zetawar

Zetawar is a turn based tactical strategy game implemented in 100% ClojureScript.
MIT License
169 stars 14 forks source link

Add water-based units #87

Closed tbeddy closed 6 years ago

tbeddy commented 7 years ago

Closes #82

This is the most involved of all Elite Command unit PRs, since it involves rule changes, and will probably require some alterations.

I added a new terrain (seaport), a new armor type (naval), and three new units (cruiser, destroyer, frigate), two of which require new unit state maps (move-attack-twice, free-attack-once). All units now have a buildable-unit parameter (a set) and all terrains now have a base-type parameter (a boolean).

I significantly altered the "base?" function and the "available-unit-type-eids" track. I moved the entire "Unit construction" section in order to use other tracks that were further down in the "subs.cljs" file.

I changed the default map to include a capturable seaport for each of the two teams. We should probably port an Elite Command map or make a new one to fully take advantage of the new units.

I included two versions of base-type in the schema, both commented out currently. Though the version I have appears to run fine, the fact that I managed to sidestep using the schema seems incorrect, but I'm not sure what changes to make.

djwhitt commented 7 years ago

Wanted to let you know, I am looking at this. It's just taking a little bit to work through it.

tbeddy commented 7 years ago

Yeah, take your time.

tbeddy commented 7 years ago

I just realized something I need to change. Base locations are defined in the scenario and I didn't change that to incorporate different kinds of bases.

Update: This has been fixed.

tbeddy commented 7 years ago

Any updates?

djwhitt commented 7 years ago

Nope, closing on a new house in the next week, so I've been too busy to make much progress. I haven't forgotten about it though. :)

On Thu, Aug 10, 2017 at 10:09 AM, tcmb notifications@github.com wrote:

Any updates?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Zetawar/zetawar/pull/87#issuecomment-321580375, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAEVpDuyijbJ8IkJffSx8zSyIE77Mlfks5sWx0qgaJpZM4OMto- .

tbeddy commented 7 years ago

Cool, just wanted to check. Congrats on the 🏠

tbeddy commented 7 years ago
  1. Does that mean it should it should use :db/valueType :db.type/ref instead of just :db/cardinality :db.cardinality/many?
  2. I did buildable-at because a TODO above the unit defs suggested_it, but can-build makes sense.

Aside from using buildable-at, is the way I revised available-unit-type-eids alright?

djwhitt commented 7 years ago
  1. Yes, that's correct.
  2. Old comment, sorry about that.

Re available-unit-type-eids, I think you should be able to remove the filter and replace it with more join conditions. Let me know if I need to clarify that.

tbeddy commented 7 years ago

I'm running into a problem with load-scenario! and the setup functions. Because terrain-types-tx runs before unit-types-tx, I can't store unit-type values in the db through :terrain-type/can-build and :terrain-build/unit-type. My solution--running terrain-types-tx twice, creating :terrain-type/can-build the second time through--is very hacky. What would you recommend I do instead? Or am I possibly using the db wrong?

tbeddy commented 7 years ago

Other issues:

I've revised the base? function (and every instance of it in the code base) to take the db as an argument and wrote a preliminary version of the function that uses a query, but I commented it out. Instead, I wrote a version similar to the earlier versions of base? that appears to work. Is there a reason why querying would be better, be it for efficiency, to be more idiomatic, or something else?

I added a new attribute to the schema, :terrain-build/unit-type. Is that overkill?

djwhitt commented 7 years ago

Regarding loading, I'm fine with using 'buildable-at' on the unit types in the definitions (not schema), and loading them using a reverse reference if that's easier.

Attribute lookup is fine for 'base?' and probably preferable for performance reasons. I should have been more careful with my wording. When I said "query" I just meant we should traverse existing relationships rather than adding a flag.

tbeddy commented 7 years ago

Do you mean going back to using 'buildable-at' in the unit types in the definitions, not using 'can-build' in the terrain types in the definitions, and converting the 'buildable-at' info into 'can-build', all during the setup phase?

djwhitt commented 7 years ago

Yeah, that's right. I think we should use 'can-build' in the schema, but 'buildable-at' is fine in the definition and will likely make it a bit easier to load. Having 'buildable-at' in the definitions also means you don't have to modify the base terrain type attributes when adding a new unit type which seems like a nice feature.

tbeddy commented 7 years ago

I realized that I was having so much trouble because I stupidly kept generating new db-ids for the terrains when I was loading 'can-build', when I could have just looked up them in the db. Thoughts on the revision I just made? 'terrain-can-build-tx' and 'build-terrains-tx' could probably be given clearer names, but I'm not sure what those should be.

Would you still like me to use 'buildable-at'?

tbeddy commented 6 years ago

Ah, I had no idea about the db issue. Switching back to :buildable-at makes total sense to me now. I'll get the revisions done in the next few days.

tbeddy commented 6 years ago

I made some revisions but I don't know if they're exactly what you were looking for. I'm still using :terrain-build/unit-type (now renamed to :terrain-can-build/unit-type). Is that the intermediate entity you're referring to? Because I can't figure out to relate both the unit-type and the terrain-type only using :terrain-type/_can-build.

djwhitt commented 6 years ago

Do you mean you don't understand how that relationship would work in the db or that you understand the relationship but don't understand how to structure the transaction to load it?

tbeddy commented 6 years ago

The former (I think). I'm currently using this to load the :buildable-at info as :can-build

{:db/id (db/next-temp-id)
 :terrain-type/_can-build [:terrain-type/game-id-idx terrain-type-idx]
 :terrain-can-build/unit-type unit-type-eid}

If I were to only create terrain-type/_can-build...

{:db/id (db/next-temp-id)
 :terrain-type/_can-build unit-type-eid}

...wouldn't the :can-build info not be associated with a terrain-type?

djwhitt commented 6 years ago

Using the current code structure, I think this is the transaction you want:

{:db/id [:terrain-type/game-id-idx terrain-type-idx]
 :terrain-type/can-build unit-type-eid}

Or you could use a reverse reference when loading the unit-type like this:

{:db/id unit-type-eid
 ;; other attributes omitted...
 :terrain-type/_can-build (map #(->> % to-terrain-type-id (game-id-idx game-id)) (:buildable-at unit-def))}

Warning: I haven't tested either of these. :grin:

tbeddy commented 6 years ago

Thanks. The former works. I was trying too hard to emulate the attack-strengths-tx and terrain-effects-tx functions and didn't think to make such a simple change. BTW, any suggestions for understanding Datomic/Datascript through and through? Just reading over the project docs?

tbeddy commented 6 years ago

Any other changes you'd like me to make?