search

ESOUIMods / LoreBooks

An Elder Scrolls Online Addon which displays map pins for Shalidor's Library books and Eidetic Memory Scrolls
Other
1 stars 3 forks source link

Update to EideticData.lua by desertforce #14

Closed desertforce closed 11 months ago

desertforce commented 1 year ago

Hi Sharlikran,

As promised here is my pull request for all the data I overhauled in the last few weeks.

My update/overhaul includes different things. In short those things are:

In addition to that I also included - by request of "oath2order" - all the data "BixenteN7Akantor" did post in the comment section for Lorebooks on esoui.com. I only left out those things I was not sure about and those which are already included in my own data. For more details please see my own comment in the Lorebook comment section on esoui.com.

For a better and more detailed overview I am attaching a .txt file to this pull request. It includes an overview of all changes I did make in the last few weeks. All those changes were tested and are working ;-) .

I will try to look into GitHub once a day for updates or questions coming from you regarding the pull request. But in case I may forget to do so or in case that I am not responding, please give me a PM on esoui.com. There I am at least once every two days looking for updates to the addons I am using!

ESO lorebooks GitHub changes.txt

desertforce commented 1 year ago

I just added something BixenteN7Akantor provided, but I thought I had already included although I had not.

Necrom (Apocrypha)

Sharlikran commented 12 months ago

6d4df129a4b39e35d8c3b870b4af070b8aceab41 has you adding coordinates to force the map name. As I mentioned that's not intended. However, now that you did that it forced me to look at things. Which isn't a bad thing per say but it's not something I really wanted to do. It was helpful though.

I looked back at the code and the original idea was to only add the map name when you are not in the same location. I don't see why the original authors chose this option. Which is why I seem to have chosen a different way of using that information.

The map name can be toggled on or off for the tooltip because it can get really long sometimes. The map name only shows when D for dungeon is present. However, that's not information you simply add because you want it there.

This is very important or it will ruin the database. You only add D when it is in the chat text. If it's not in the chat text then don't add it. Don't add D to a fake pin to make it do what you want either.

Providing lorebook data isn't to enter it so it looks nicer as a quality of life kind of thing. As I mentioned in the forums use the text from chat as it appears for the most part. Then add what is needed from there as long as it applies.

Sharlikran commented 12 months ago

I can see the majority of this is usable. However, it's going to be a while. I don't want to say how long before I get to it. So if you want to add other data that's fine also. Just make it a different pull request.

Sharlikran commented 12 months ago

I do need to figure out how to get this entered though. There is quite a lot that is useful. I appreciate the work.

desertforce commented 12 months ago

Hi Sharlikran,

Thank you for your response.

If I may, just a few things for a better understandig for me :-) .

You wrote that in "6d4df12" the coordinates do force the map name and that the "d" should only be used when it really is given by the /lbpos function from the addon, meaning that the book is really in a dungeon/instanced zone. I just checked the Version 72 file against mine with all the changes. If I am not mistaken I did not enter any "d" tags on my own, since I do know that you once told me to only use what I get with /lbpos and /lbfake and that is what I did. Basicly there were only three things I did.

  1. The first being changing the order of the code for some books, so that it fits the order used atm for a better overview. That order as far as I was able to see is --> ["c"] , ["cn"] , ["n"] , ["r"] (if needed) , ["q"] (if needed) and ["e"] .

  2. The second being copying the part from the data I collected with the addon through /lbpos and /lbfake into the EidecticData files, which is needed for the code. As you once explained to me that is everything starting with the [1] {.... and going until the first " }, " . That first " } " may come after the "pm" , "d" or "fp" tags, depending on what /lbpos and /lbfake returned regarding the book location.

  3. The third thing I did was consolidate some book locations/entries ( [1] , [2] , etc.). For example, for the book [7678] I took the [2] location/entry, which was a "fp" (Fake-Pin) and added it to the [1] location/entry as zone-coordinates. The reasons for that were two things:

A) You told me once that all books which are not on the zone-map, but below it on another map, do need Z-coordinates. Those z-coordinates I will get through creating a fake-pin and then changing the fake-pin p-coordinates into "z" ones. The only exceptions for this can be cities or locations where px/py are nearly identical to the zone-map (fake-pin) coordinates zx/zy. There I can just add "zm" and leave "zx" and "zy" out.

B) By consolidating them the addon shows the location in the tooltip for a better grasp where the book is located.

Now one question for me would be. Should this sort of consolidation not be done, because if it is done, especially for all the books with a "d" tag in their positions, the addon will show the location in the tooltip, which is not a good thing since this function isn't working as you want it to work? Depending on the answer to that question a follow up question would be. Should all books with the "d" tag get an extra entry for a fake-pin instead of adding that via the "z" - coordinates where it is possible or not ? --- As an example take book [7678] , which has 2 entries in Version 72 ( [1] = original position where the book is , [2] = fake-pin for zone-map location) and after my changes it now only has one entry [1] , which includes the real position with p-coordinates and the zone-map position with z-coordinates. Should such books be left as they are and compareable new books added also with two entries (as this one had) or should it be done as I did?

Sorry if those are stupid or unreasonable questions. But I am asking them because I would like to know how to better handle possible contributions in the future. Also, I am asking this because I would like to know if I can still keep up with my goal to provide lorebook data so that books in cities won't only be shown on the city-map or the world-map, but instead be shown on BOTH maps ;-) .

P.s.: Perhaps for the next data I will provide I should find a better phrasing for "overhauled code so that location is shown in world-map tooltip". Maybe I should just write "merged entries A and B" or "added z-coordinates" and leave the part with "location is shown in world-map tooltip" out so that there won't be any confusion :-) .

desertforce commented 12 months ago

I can see the majority of this is usable. However, it's going to be a while. I don't want to say how long before I get to it. So if you want to add other data that's fine also. Just make it a different pull request.

Sorry for asking, but since I am still learning how to use GitHub, what must I do to open up / create a different pull request???

desertforce commented 12 months ago

I do need to figure out how to get this entered though. There is quite a lot that is useful. I appreciate the work.

Thank you for the appreciation. Since I really like this addon and since you are doing such great work, I also tried to do my best. I do really hope you will be able to find a way to use as much data as possible, so that there will be less collection work which needs to be done =) .