Add water-based units

[tbeddy]

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.

[tbeddy]

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]

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]

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]

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]

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'?

[djwhitt]

There are some issues with this approach that I mention below. They could be fixed, but I think things will still be simpler overall if we go with buildable-at in the the unit-type definitions.

[djwhitt]

Terrain types should be related directly to unit types without an intermediate entity. Also, the find-by query will only work if there is a single ruleset loaded into the db. We want to be able to support multiple rulesets being loaded simultaneously.

[tbeddy]

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]

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]

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]

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]

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. 😁

[tbeddy]

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]

Any other changes you'd like me to make?

[djwhitt]

Just one minor change and then I think this is good to go.

[djwhitt]

This should use the to-terrain-type-id function.

[tbeddy]

Done