sander1095 / DnDCombatTracker

A D&D 5e Combat Tracker made using Windows Form
MIT License
19 stars 9 forks source link

Rewrite everything with a pattern (WPF) #5

Open sander1095 opened 7 years ago

sander1095 commented 7 years ago

Maybe use WPF and MVVM of some sorts to make the code follow a pattern and thus make it easier to develop new features.

Also make it so that we can unit test this thing.

ihildebrandt commented 7 years ago

I started on a WPF Version. Please feel free to grab my branch and tell me what you think.

DnD Combat Tracker Gif

It's definitely not feature complete yet, but I have most of the Create/Edit workflow.

I based it off of master, so I don't have any of the work that you've done on the menu or About page. It won't be difficult to work that in, though.

https://github.com/ihildebrandt/DnDCombatTracker/tree/feature/wpfapplication

sander1095 commented 7 years ago

Hello @ihildebrandt !

Thank you very much for helping me build this application. You're the first contributor so I am very excited!

I do not have time to look at it now because I'm in the last week of my graduation but I'll surely take a look at it soon.

Just a couple of questions:

  1. Will you make it feature complete (to the point where it is now on the Master)?

  2. I see in the gif that some of the "flow" has changed. I assume you use MVVM or something that uses INotifyPropertyChanged. Is there a reason you have decided to change the workflow? I can see that a user now has to press "create" to create a new character. In the previous version the user can edit an existing character and if the name is different it will count as a new one (changing the name still has to be added and is an open issue). This saves the user a couple of clicks/tabs. Why havd you chosen this and how will this affect other features?( For example, in your version a user could edit an existing character but would have to press the Create button to save it. This is confusing for the user.

I also see you have added in the Roll functionality(#2) and Turn functionality (#23). Will you make these fully functional? Please take a look at the new description of #2 and tell me what you think.

Thank you so much for contributing to this application!

One last thing. I am looking into rewriting everything to UWP or using the Desktop Bridge to allow this application to be submitted to the Windows Store. I do not want to do that soon because more features still have to be added but I think it might be a cool thing to add. What is your opinion on this?

ihildebrandt commented 7 years ago

No worries about not being able to look at it. Congrats on graduating. I just wanted to go ahead and let you know that I was on it so that we don't work over one another. I absolutely intend to keep working on it until it's feature complete.

As far as the work flow is concerned, yes, It's using an MVVM pattern with INotifyPropertyChanged. This causes the record to be automatically updated whenever the user changes the form. There is no need to press 'Create' when updating an existing record (the gif wasn't really clear on that). Create only needs to be pressed when a new record is being added. The way the previous app works (changing the name saves a new record) doesn't quite mesh with an MVVM workflow, though it may be possible to get it to work that way, I'll play with it and see if I can get it closer to the current functionality.

The 'Roll' function for initiative was just kind of tacked on at the end. I was playing with a DieRoller class that will take traditional DnD style dice strings (2d8 + 1) and roll them. I will certainly take a look at fulfilling the issue.

I think having a UWP app is a great idea. One of the things that I am doing with this app is to move all of the business logic into a separate assembly with only the UI controls in the App. That way you would be able to create a UWP version that will scale properly across multiple devices and still have the WPF version for people not on 10.

sander1095 commented 7 years ago

@ihildebrandt Hello! I can confirm I graduated. Yay :)

I gave UWP some more thought and I think it's a bad idea to use it because I would only support Windows 10. WPF would suffice I think and thus I suggest sticking with that. Maybe a seperate branch could be used in the future to maintain a UWP version, or the WPF version could be ported to UWP so it could be placed in the store.

I'd like to keep working on this application during the summer!

herrozerro commented 5 years ago

Here are my proposed changes:

I could take the lead on pulling everything into the service, or at least the first stab.

let me know what you think.

sander1095 commented 5 years ago

That sounds pretty good to me! I don't have a preference on how you want to start this; if you would explain in this issue how you wanna program it/what your vision it and get started, that's fine with me. Others could then help out later or continue on your work.

I think it could also be cool to start targeting .NET Core 3.0 at some point? :) I don't know if it's beneficial but hey, it's the new stuff 😋

herrozerro commented 5 years ago

I agree about .net core 3, it's actually one of the reasons why I made the update to 4.6.1 so I could have the core library be .net standard so we could support core and possibly core 3.

sander1095 commented 5 years ago

Sweet :)

Perhaps we could also look into #6 . I see that I started working on this issue in the dev branch but never continued. I suppose I should have created a feature branch for this.

The reason why I am thinkin about this is, is because I wanna figure out the best way to merge your PR. Perhaps merging it into develop first for testing purposes and all that would be a better way than merging it to master?

herrozerro commented 5 years ago

Next, if you don't mind, along the lines of this issue I think I could take a stab at removing the core functionality from the UI and making a combatmanagerservice in the core project.

sander1095 commented 5 years ago

Sounds good

herrozerro commented 5 years ago

I did some research into .net core, and the first major thing that will change is that clickonce is not supported with .net core.

That's not to say that an auto-updating application is out of the question, but it might have to be done manually. This app is an example: https://github.com/KSP-CKAN/CKAN. The basic idea is that you manaully do the version checking. If an update is found download the files and in this case an autoupdate.exe.

Then trigger the auto updater that takes the location of the app, the location of the downloaded files and the process id of the app. The auto updater kills the app if needed, copies over the system files and then starts up the program again.

Or you could just stick with .net 4.x and use .net framework wpf. Clickonce is still supported.

sander1095 commented 2 years ago

Perhaps it'd be good to look into .NET MAUI? I don't think we need to focus on Android/IOS support since that wasn't the original scope for this project.

I think nowadays different initiative trackers are available, perhaps even better ones. But rewriting it in MAUI could be a good way of practiicng that framework.