AnthonyZJiang / D-OS-Save-Editor

A save editor for Divinity Original Sin: Enhanced Edition
Other
50 stars 15 forks source link

Add Abilities Effect #8

Closed Greavox closed 6 years ago

Greavox commented 6 years ago

sans-titre-1 The DataTable file was modified by adding a "string array" wich each ability grouped by 3 like this :

  1. Name of Variable that used like Bow or SingleHanded
  2. Name of Ability like InGame example Bow or Single-Handed
  3. Description of Ability like InGame example "Bow increases the damage you do using bows."

UpdateForm() and SaveEdits() from AbilitiesTab were reduced to become more dynamically with DataTable "like" TalentsTab. To do that, each TextBlock received a X:Name.

Unknown Abilities were keeped PlaceHolderX (X is int between 2 and 5 included) : I don't see difference in Player Info when I select them in game

AnthonyZJiang commented 6 years ago

Regarding softcode vs hardcode, I want to express my 2 cents here (I'm a casual programmer too):

Softcode looks elegant and is a joy to program it, and I like this code style in many occasions when flexibility/extensibility is needed or hard coding is cumbersome. However, here, flexibility and extensibility are not required and hard coding is not that difficult*. Hardcode, in my opinion, a) is faster in terms of performance, b) less prone to error and c) reads well.

Also, I try to avoid defining x:name, otherwise, in Intellisense, I could be reading a ton of variable names. If I were to soft code this, I would define the Uid or Tag properties in xaml. Uid is works the best for me as I can define a lot of things in form of a delimited string; then I can access them by splitting the string with the delimiter. Tag property takes an object, so it is better to be used with a class or struct to pass complex information in code behind or MVVM.

But since you have soft coded it and it's a good job, let's keep it then. We are not going to extend it anyway 😄

* I got these ability names from a post on Larian's forum, and I can paste them in an Excel spreadsheet. I then I can easily generate all these abilityNameTextBox.Text = ... and <TextBox x:Name="abilityNameTextBox"... codes under a few minutes.

Greavox commented 6 years ago

You right!

Ability class isn't used and I forgot to delete it... Sorry

Thank you very much for your review, it's help me! I'm newbie in C#, begin with your project (PHP with a very little JS are only programming language I know and use) and C# is a powerful tool with too much reference for my little brain 😆

I'll rewrite code to a more hard coded version, well if I can do it ^^

But now, I'm going to bed too. Good night

AnthonyZJiang commented 6 years ago

Thanks! I hope my review and comments do not sound picky to you. On the contrary, I really appreciate your contribution. I'm always glad to see collaborating and inputs from other, which make the project more interesting and lift the amount of work on me 😈.

Greavox commented 6 years ago

Don't worry, I'm learning ^^

Greavox commented 6 years ago

And now?

AnthonyZJiang commented 6 years ago

Wow you are amazing. It's been a busy night for me. I'm going to check your code tomorrow morning ;) thanks dude!

AnthonyZJiang commented 6 years ago

Thanks!!! Really hard work and well done!

Greavox commented 6 years ago

I'm follow the Master XD

Xeddicus commented 6 years ago

On line 185 of AbilitiesTab.xaml for the textblock for Tenebrium it reads "...get..." instead of "...gets..." or probably even better "...gets the...". My contribution to this awesome work. :P

AnthonyZJiang commented 6 years ago

On line 185 of AbilitiesTab.xaml for the textblock for Tenebrium it reads "...get..." instead of "...gets..." or probably even better "...gets the...". My contribution to this awesome work. :P

Fixed! Thank you!