search

cs96and / FoundryVTT-damage-log

FoundryVTT module to report all damage and healing to a separate chat tab
MIT License
9 stars 9 forks source link

[DRAFT] SPIKE - Switch from System Config JS Objects to Class #22

Closed ProfDoof closed 1 year ago

ProfDoof commented 2 years ago

(Fair warning, my background is pretty heavily in C# so I tend to bring some of those opinions into my JS code)

I decided that I didn't want to stare at my research anymore so I decided to take a break and code up this spike. It's just an idea but it should let you clean up all of these things further if you like any of it. For example, might be able to take some of these attribute calculation and generation stuff and abstract it out to the Attribute class.

I haven't had the opportunity to test it because I don't have my own foundry instance and didn't want to bother my DM about it at nearly midnight but the logic is decent enough though I will admit to not being as confident in my JS abilities as I am other languages.

cs96and commented 2 years ago

Thanks for this. However, I'm struggling to see what the intended purpose is, or what problem it solves.

The existing code had all the attributes defined in a nice set of structures, but the new code abstracts that all out into a bunch of addAttribute calls, that make it much more difficult to see at a glance how each system is configured.

ProfDoof commented 2 years ago

My thought was that you can use this to ensure proper structure when you get around to implementing a settings page. It also means you can abstract out the calculations to the Attribute itself. Finally, the real benefit to me anyway is that you can have each attribute themselves know how to display themselves.

However, like I said, a lot of my opinions are informed by more statically typed OO languages so the way I approach problems in them is typically in that way.

Anyhow this only took me a couple of hours so if you see anything you like feel free to take it but otherwise it's no big deal.

On Mon, Jul 25, 2022, 8:29 AM Alan Davies @.***> wrote:

Thanks for this. However, I'm struggling to see what the intended purpose is, or what problem it solves.

The existing code had all the attributes defined in a nice set of structures, but the new code abstracts that all out into a bunch of addAttribute calls, that make it much more difficult to see at a glance how each system is configured.

— Reply to this email directly, view it on GitHub https://github.com/cs96and/FoundryVTT-damage-log/pull/22#issuecomment-1193987391, or unsubscribe https://github.com/notifications/unsubscribe-auth/AH52FUDET66YWTL3A2WUP5TVV2CD7ANCNFSM54QV6XDA . You are receiving this because you authored the thread.Message ID: @.***>