-
Notifications
You must be signed in to change notification settings - Fork 9
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
Rewrite everything with a pattern (WPF) #5
Comments
I started on a WPF Version. Please feel free to grab my branch and tell me what you think. 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 |
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:
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? |
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. |
@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! |
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. |
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 😋 |
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. |
Sweet :) Perhaps we could also look into #6 . I see that I started working on this issue in the 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? |
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. |
Sounds good |
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. |
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. |
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.
The text was updated successfully, but these errors were encountered: