FFXIVAPP / ffxivapp-plugin-parse

Visit us on Discord! https://discord.gg/aCzSANp
https://xivapp.com
MIT License
6 stars 12 forks source link

Feature request: Job indicators on score cards #17

Closed joelspadin closed 9 years ago

joelspadin commented 9 years ago

It would be nice to have some indication (job icon, job-specific colors, etc.) of what job each player is on the score cards. I always have to keep looking back and forth between the game screen and FFXIVAPP to match up names to jobs. It would also be helpful to sort the list by role. My card seems to show up in a random spot in the list, making it difficult to find myself. I would like to be able to sort similar to the in-game party sort order, or at the very least have my card always appear first.

I attempted to make some of these modifications myself, and I have been able to compile all the ffxivapp-* projects. When I run the FFXIVAPP client by itself, it seems to work properly, but if I drop my compiled version of FFXIVAPP.Plugins.Parse.dll into the Plugins folder, it starts throwing hundreds of exceptions in the localization code, such as:

System.Windows.Data Error: 17 : Cannot get 'Item[]' value (type 'String') from 'Locale' (type 'Dictionary`2'). BindingExpression:Path=Locale[parse_TotalsNormalHeader]; DataItem='PluginViewModel' (HashCode=27223978); target element is 'TextBlock' (Name=''); target property is 'Text' (type 'String') TargetInvocationException:'System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.Collections.Generic.KeyNotFoundException: The given key was not present in the dictionary.
   at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
Icehunter commented 9 years ago

I have a AIO project file I setup to have all the projects in one solution. What I would recommend is grab ffxivapp, ffxivapp-resources and this plugin.

I then add them all to a new solution and make sure the plugin is referencing the resources folder. I would think that error is from not having access to correct DLL version.

I've bene playing around with job resolution (for party it's okay) but it doesn't work so well with alliance as I haven't found their info in yet as a structure. Never enough time to make them wait in-game while I hunt it down.

Let me know if that works for you. If not I might just have to upload the solution file and you open it in notepad and change all the instances of my path locations to yours.

joelspadin commented 9 years ago

I tried making my own solution, used git submodules to pull in all the pieces of ffxivapp (common, iplugininterface, localization, etc.), then set each project to reference the other projects in the solution, and I still got the same error. I'll try referencing the versions from ffxivapp-resources next.

joelspadin commented 9 years ago

Nope. Still the same problem.

joelspadin commented 9 years ago

It looks like it might be trying to load the plugin UI before setting up the localization dictionaries. If I put a breakpoint in LocaleHelper.Update(), it breaks immediately upon starting FFXIVAPP.Client if I have no plugins, but if I add the parse plugin, I get a never ending stream of exceptions instead.

joelspadin commented 9 years ago

Well, I suppose the stream of exceptions is only never ending if I attach a debugger. If I'm not debugging with Visual Studio, it pretty quickly loads the plugin and associated locale text.

joelspadin commented 9 years ago

I have no idea if this is the right way to solve this, but I stuck LocaleHelper.Update(Settings.Default.Culture); into Initializer.LoadPlugins() right after it calls App.Plugins.LoadPlugins(), and the key not found exceptions all went away. There are still a bunch of null reference exceptions and binding expression path errors, but at least the client appears to load quickly and properly now.

Icehunter commented 9 years ago

It's possible the code you have for the parser and other items are out of date? I recently updated the localization resolution of all plugins and each one now contains the localization for itself.

I have that line in Plugin.js called this way now:

https://github.com/Icehunter/ffxivapp-plugin-parse/blob/master/FFXIVAPP.Plugin.Parse/Plugin.cs#L88

Try back-merging master into your branch after fetching all the updates and see if that resolves it?

joelspadin commented 9 years ago

I haven't actually forked your code yet. I pulled the latest code from each repository today.

I think I see what is going on now though. When plugins are loaded, the call stack looks like this:

FFXIVAPP.Client.AppBootrapper.Instance.get()
FFXIVAPP.AppBootstrapper.AppBootstrapper()
FFXIVAPP.Client.Initializer.LoadPlugins()
FFXIVAPP.Client.Helpers.TabItemHelper.LoadPluginTabItem()
FFXIVAPP.Plugin.Parse.Plugin.CreateTab()
FFXIVAPP.Plugin.Parse.ShellView.ShellView()
[External Code]
FFXIVAPP.Plugin.Parse.PluginViewModel.Locale.get()

at this point (PluginViewModel.cs line 64), _locale is still null, so it gets initialized to an empty dictionary. Nothing has called the setter for Plugin.Locale (Plugin.cs line 85) yet, so it hasn't called LocaleHelper.Update() and it hasn't set PluginViewModel.Instance.Locale to the filled-out dictionary yet.

So in short:

  1. The application starts
  2. The application loads all plugins
  3. The application initializes the UI for all plugins
  4. The plugin UI fails to bind locale strings because Locale is still an empty dictionary. (If you are debugging with Visual Studio, wait 10 minutes as it notifies you of each and every missing string in the UI.)
  5. At some later time Locale is filled out and the UI updates properly.
Icehunter commented 9 years ago

Is there a recommended solution? Debugging has been the bane of my existence after moving to the plugin system vs everything coded if I'm being honest.

I know the application loads it's own UI (it used to load all of them I believe) here:

https://github.com/Icehunter/ffxivapp/blob/master/FFXIVAPP.Client/ShellView.xaml.cs#L80

Perhaps in each plugin the local should be loaded before the TabItem is created?

https://github.com/Icehunter/ffxivapp-plugin-parse/blob/master/FFXIVAPP.Plugin.Parse/Plugin.cs#L160

Maybe the "get" could default to actually loading (if null) the basic one:

https://github.com/Icehunter/ffxivapp-plugin-parse/blob/master/FFXIVAPP.Plugin.Parse/Plugin.cs#L84

However this always gets called and this is also where the initial language get's set, causing a change reaction.

https://github.com/Icehunter/ffxivapp/blob/master/FFXIVAPP.Client/Helpers/LocaleHelper.cs#L76

I'll go test out some changes right now to make it easier :)

Icehunter commented 9 years ago

So after testing and watching the debug window open (no game open) in less than a few seconds this works:

        public TabItem CreateTab()
        {
            Locale = LocaleHelper.Update(Constants.CultureInfo);
            var content = new ShellView();
            content.Loaded += ShellViewModel.Loaded;
            var tabItem = new TabItem
            {
                Header = Name,
                Content = content
            };
            //do your gui stuff here
            EventSubscriber.Subscribe();
            //content gives you access to the base xaml
            return tabItem;
        }

So I'll start updating each plugin and the samples and then that will fix it :D

joelspadin commented 9 years ago

I'll try that. That seems better than my hack of having the plugin loader force a locale update on the plugin.

I managed to get job icons working (at least for my character, I haven't tested with a party yet). I had to mess around with a bit of existing code, since the UI wasn't getting notified on changes to Player.NPCEntry.

Icehunter commented 9 years ago

I pushed it up and updated all plugins and released, for the job icons I can help out with that as well if you want.

joelspadin commented 9 years ago

My changes are pretty minimal. I added job icons into the resources, a converter to turn an Actor.Job into the appropriate icon, added an image bound to NPCEntry.Job in PlayerInfoBox.xaml, and then made the aforementioned hack to give Player access to StatGroup.RaisePropertyChanged() to let it notify on changes to NPCEntry. There's probably a better way to handle that last bit.

I'll probably clean up my code and submit a pull request sometime tomorrow.

Icehunter commented 9 years ago

Awesome!