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 [Auridon & Couldharbour] (by desertforce) #16

Closed desertforce closed 9 months ago

desertforce commented 11 months ago

Hi Sharlikran,

I completed my collection for some more lorebook data in the last few weeks and just finished entering and testing it. Therefore here my pull request with the new/overhauled data.

This pull request includes the following new/overhauled data:

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

Just in case, I also want to apologize beforehand, because this update will be again a bit larger. Like the last one. It includes changes and overhauls to around 200+ lorebooks. So, sorry about the huge amount of data ;-) .

ESO lorebooks changes (Auridon & Couldharbour).txt

desertforce commented 11 months ago

Supplement...

I just noticed that I forgot to enter the location information for the Oct 16, 2023 changes.

Oct 16, 2023 Couldharbour

Sharlikran commented 9 months ago

I am still going to use this but I mentioned already to you and @PorygonA that I want to be able to review the changes. PorygonA had organized the books once by ID and that was fine but it's so easy to search for a book ID and then jump to in in Notepad++. When I'm adding a book or refining a book it takes me a few moments to find and edit the book and update it.

So I'll just leave the comment here. In the future, I won't be able to accept the pull request if it's cosmetic in nature to reorder things just so it looks pretty.

The reason is that I'm contemplating writing some code to change the information. The game pauses too much when switching zones because the code loops over thousands of books and then there is a loop for the book to see if any locations apply to that zone. It's not efficient. I haven't done it because I may resolve the issue a different way.

If I do output the books in a slightly different format I probably wont change the markers like ["c"], ["cn"]. The code would output everything to the saved variables file and once that happens ZOS does not allow you to specify the order it is written. This means all the books will be out of order again with book 4096 before 301 and the next book could be 7000. Then the fields will be however ZOS saves them. I have no control over the output.

Prettiness and origination is not important. Having accurate book locations and notes for them when needed is all that's important.

Sharlikran commented 9 months ago

Another thing I need to leave for the both of you here so I don't have to post in multiple places is that commits can't have multiple books. Which is why cosmetic changes like reordering the fields is not going to be accepted. Once I merge PorygonA's changes first, this won't merge without a complete interactive rebase.

Sharlikran commented 9 months ago

And also obviously I'm not trying to be a jerk and discourage anyone from contributing, but the reorganization creates commit noise that doesn't need to be there. It can be done but that needs to be discussed and added as one commit so people can then rebase without spending hours doing so.

Sharlikran commented 9 months ago

The main reason there will be so many conflicts is that PorygonA's branch has not been rebased since version 70 and the earliest commit is from June.

Sharlikran commented 9 months ago

Another reason for these comments is because I may go 2 or three months before merging anything so the commits needs to be done in a way that is easy to rebase.

Sharlikran commented 9 months ago

PorygonA's Entry

  [6950] = {
    ["c"] = true,
    ["cn"] = "Systres Tomes and Scrolls",
    ["n"] = "Ode to the Nose of a Woman",
    ["q"] = 6760, -- it is given to you at the begining of the quest
    ["e"] = {
      [1] = { ["px"] = 0.0518611997, ["py"] = 0.5992903875, ["pm"] = 2219, ["d"] = true, ["zm"] = 2114, ["qc"] = true, }, -- can be found here AFTER quest is completed
    },
  },

Your Entry

[6950] = {
    ["cn"] = "Systres Tomes and Scrolls",
    ["n"] = "Ode to the Nose of a Woman",
    ["q"] = 6760,
    ["e"] = { --Does not have a location; it is given directly to your inventory in the quest and must be read.
    },
    ["c"] = true,
  },

So don't forget to consider this situation which is why having 1000 book changes per commit really isn't going to work in the future.

Sharlikran commented 9 months ago

And don't rebase this I can see I have to spend hours and hours to redo every book from these commits one book at a time because of things like this.

image

Where the current version has updated information and your commit doesn't because you have combined so many cosmetic changes to the books by reordering the fields and changing too many books in one commit.

desertforce commented 9 months ago

I am still going to use this but I mentioned already to you and @PorygonA that I want to be able to review the changes. PorygonA had organized the books once by ID and that was fine but it's so easy to search for a book ID and then jump to in in Notepad++. When I'm adding a book or refining a book it takes me a few moments to find and edit the book and update it.

So I'll just leave the comment here. In the future, I won't be able to accept the pull request if it's cosmetic in nature to reorder things just so it looks pretty.

The reason is that I'm contemplating writing some code to change the information. The game pauses too much when switching zones because the code loops over thousands of books and then there is a loop for the book to see if any locations apply to that zone. It's not efficient. I haven't done it because I may resolve the issue a different way.

If I do output the books in a slightly different format I probably wont change the markers like ["c"], ["cn"]. The code would output everything to the saved variables file and once that happens ZOS does not allow you to specify the order it is written. This means all the books will be out of order again with book 4096 before 301 and the next book could be 7000. Then the fields will be however ZOS saves them. I have no control over the output.

Prettiness and origination is not important. Having accurate book locations and notes for them when needed is all that's important.

Ok, I do understand that. Spimplicity with such huge amounts of code is a must have. Therefore, please let me try to see if I am able to break it down to the essential things:

Are those basics correct?

desertforce commented 9 months ago

Another thing I need to leave for the both of you here so I don't have to post in multiple places is that commits can't have multiple books. Which is why cosmetic changes like reordering the fields is not going to be accepted. Once I merge PorygonA's changes first, this won't merge without a complete interactive rebase.

I am not 100% sure I do understand what you mean with ...commits can't have multiple books.. Do you mean, that there should be only ONE change to ONE book per commit??? That would mean that there would be a flood of commits when someone (like me) is going through a whole Zone. Or do you mean, that there should be only ONE PERSON doing a commit for ONE BOOK and not multiple persons? In that case we could run into a problem regarding the communication between the people who do commit stuff and what they did, are doing or will be doing. I would be happy if you could/would clarify this for me a bit more please =) .

The main reason there will be so many conflicts is that PorygonA's branch has not been rebased since version 70 and the earliest commit is from June.

If I understand this correctly the problem is that not all of us are up-to-date on the same level, regarding the file versions and its contents. Perhaps we could find a way to make this in an easy way possible? I for example have had some problems too with updating my branch, mostly because of I am new to Git and didn't knew how to do it. Now I do have a main branch from which I creat sub ones, which I do use to make commits. The main one I don't touch and use it only to update with your main one, so that its up to date.

Another reason for these comments is because I may go 2 or three months before merging anything so the commits needs to be done in a way that is easy to rebase.

Depending on what you did mean regarding what you wrote in your other comment (...commits can't have multiple books.), I do have a question here. Since it should be easy to rebase, should there be a maximum amount of books, which should be included per commit - for example not more than 100???

desertforce commented 9 months ago

PorygonA's Entry

  [6950] = {
    ["c"] = true,
    ["cn"] = "Systres Tomes and Scrolls",
    ["n"] = "Ode to the Nose of a Woman",
    ["q"] = 6760, -- it is given to you at the begining of the quest
    ["e"] = {
      [1] = { ["px"] = 0.0518611997, ["py"] = 0.5992903875, ["pm"] = 2219, ["d"] = true, ["zm"] = 2114, ["qc"] = true, }, -- can be found here AFTER quest is completed
    },
  },

Your Entry

[6950] = {
    ["cn"] = "Systres Tomes and Scrolls",
    ["n"] = "Ode to the Nose of a Woman",
    ["q"] = 6760,
    ["e"] = { --Does not have a location; it is given directly to your inventory in the quest and must be read.
    },
    ["c"] = true,
  },

So don't forget to consider this situation which is why having 1000 book changes per commit really isn't going to work in the future.

Regarding the code re-organisation, as you wrote in your other comment, that is something I need to leave out for now. If done, it should be first agreed up on a certain order and then be done in one swift effort for all books, so that we do have one single syntax form for all of them.

Regarding the example you gave. Just for protocol. This is a good example for miscommunication between us (you, pory (oath) and me). The info for that book was provided by "BixenteN7Akantor" through the comment section at esoui.com for LoreBooks. Since oath2order (I assume this is "PorygonA") had no time dealing with those infos, he asked me if I could do that. Because I was, at that time, prepairing my own info for a commit, I said yes and included it. Here down below are the dates and the time you can find the comments in the commentsection for LoreBooks at esoui.com.

09/12/23, 12:50 AM ... desertforce

09/11/23, 09:07 AM ... BixenteN7Akantor

09/10/23, 10:28 PM ... desertforce

09/10/23, 09:17 PM ... oath2order

As far as I can remember, the last code example (the one under Your Entry) was the original one included in the code. I don't know who wrote that, but I am sure it was not me. What I do know is that I entered the infos provided by Bixen and commited the change. The result was the code as seen as in your first code example. If PorygonA also commited those changes, he must have forgotten about what he wrote the commentsection. In addition to that another thing I know is that I have seen 2 or 3 more of those - lets call them - "semi-entries" in the code, while updating books. I think one of them I updated with my last commit for Auridon and Coldharbor.

Btw., now that I have been looking at the example for some time, I do think you may have mixed up the headlines (^v^). That could mean PorygonA was possibly the one writing those semi-entries Oo ?

desertforce commented 9 months ago

And don't rebase this I can see I have to spend hours and hours to redo every book from these commits one book at a time because of things like this.

image

Where the current version has updated information and your commit doesn't because you have combined so many cosmetic changes to the books by reordering the fields and changing too many books in one commit.

I see and understand what you mean. The trouble I do have is, that there are at least 3 or 4 different ways of syntax forms in the file and I don't know should I only follow one of them, some of them or all of them. Here the thing you mentioned, regarding the form and organisation of the file, comes into play. Without a more or less clear form to follow you may get lost.

Could or would there be perhaps any way to decide upon a certain form to follow? For example, use the form of the last entries, the ones from the books with IDs [7xxx]?

  [7776] = {
    ["c"] = true,
    ["cn"] = "Apocryphal Pages",
    ["n"] = "Journal of the First Remnant",
    ["q"] = 6992,
    ["e"] = {
      [1] = { ["px"] = 0.1566891944, ["py"] = -0.0297452037, ["pm"] = 2339, ["d"] = true, ["zx"] = 0.1537979973, ["zy"] = -0.0282487988, ["zm"] = 2275, ["qp"] = true, },
      [2] = { ["px"] = 0.1537979973, ["py"] = -0.0282487988, ["pm"] = 2275, ["fp"] = true, ["qp"] = true, },
    },
  },

Perhaps making it a bit easier to view for you to review.

  [7776] = {
    ["c"] = true,
    ["cn"] = "Apocryphal Pages",
    ["n"] = "Journal of the First Remnant",
    ["q"] = 6992,
    ["e"] = {
      [1] = { ["px"] = 0.1566891944, ["py"] = -0.0297452037, ["pm"] = 2339, ["d"] = true, 
              ["zx"] = 0.1537979973, ["zy"] = -0.0282487988, ["zm"] = 2275, ["qp"] = true, },
      [2] = { ["px"] = 0.1537979973, ["py"] = -0.0282487988, ["pm"] = 2275, ["fp"] = true, ["qp"] = true, },
    },
  },
Sharlikran commented 9 months ago

PorygonA was possibly the one writing those semi-entries

Could have been anyone I normally don't add comments.

commits can't have multiple books

Yes they should be one at a time unless you are adding changes to 3 in succession. For example 3000, 3001, 3002 and you are changing or adding all three. Because then if I do an interactive rebase with conflicts and could exclude that commit rather then trying to diff thousands of changes like this.

I don't want to make anything written stone though. I will explain later at the bottom.

image

Could or would there be perhaps any way to decide upon a certain form to follow

No. The reason is that depending on what you are doing, and how your mind or in your mind how it is going to work then do what works. That's why the other commit I had to go back and look at the code. You wanted to change the format because you felt it would make the pin show differently. I forget maybe it was the tooltip? So your commit had comments like "commit to make it show the zone name" or whatever it was. I had explained then that the tooltip is however I wrote it at the time and the tag such as ["??"] is what makes the difference, so don't worry about it.

I had one person deliberately put books on two lines because he was overhauling a city of say 20 books and it was easier for them to copy paste some information for it in two lines. I didn't care for that but in the end it didn't matter.

I don't want to go back and forth about all of this really. The main thing is when I don't have time to do anything with this for 2, 4, 6 months then when I go to do it I want to be able to quickly merge in what will work and not have to spend several days and several hours per day diffing in changes.

So having only a few books per commit maybe I can skip some or Git will simply handle it by itself. Also maybe if there is a conflict it could be two sections out of 30 or 40 and it's just marked.

>>>> HEAD
<<<
>>>

I forget how it does it but Git will mark the conflicts and maybe it will be easier to navigate.

I normally change 10 or 20 books in a version and I'll just plop all 20 down in one commit sure. If you are overhauling hundreds of books or thousands of books then I need to see the changes easily because if I just merge your current 3 commits now I would actually revert changes already made.

You mentioned maybe doing one zone at a time. That could be one way. Maybe 20 books for a city, 20 books for another city and two different pull requests, and 50 for the overland and all three are for the same zone. Something to break it up.

Because like I said, if I choose to redo some of the format to eliminate the long pause when I export the Lua code to saved variables it will put it in any random order. I can't control that. I have not decided what to do yet of course I'm just saying that's why worrying about a specific order isn't really needed for whether or not C is first or whether C or CN is next and so on.

Things will always be contradictory for me because in the end it's how much time do I have to spend to prevent reverting changes between multiple people.

Looking at some of the changes I worry lines are being removed because another line is more favorable or someone thinks that the book is here and not over there. I don't want anything removed, just update what looks the closest if there are multiple old book entries. I even look at each change I make and if I think there were two entries and one was the fake pin then I'll remove the two entries and put one line. I think you have been doing that.

However, with BixenteN7Akantor I feel like maybe to some degree they make suggestions like "It's here not there". We know that for some books there may be more than one location or it's in one location while on the quest and another after you complete it. There could be old outdated information and I don't want to remove it unless there is some way to verify if it's defiantly not there anymore.

And lastly, I'm just not with the caring of writing an essay and white papers on things. If you want update some books just change.

      [1] = {
        ["mn"] = 28,
        ["py"] = 0.5259800000,
        ["px"] = 0.2527900000,
        ["pm"] = 994,
      },

To

    ["e"] = {
      [1] = { ["px"] = 0.2528356070, ["py"] = 0.5259532177, ["pm"] = 994, },
    },

And go on with your day. I don't need restructuring, ordering, syntax conformity, and other OCD nitpicky stuff.

At the end of the day I just want to just say thank you for the updates, and upload the new version.

Sharlikran commented 9 months ago

So thank you for the updates it will take a while to get this all sorted. I appreciate it.

Sharlikran commented 9 months ago

I forgot with the quests that plop a lore book in your inventory. Do not make some kind of fake entry for them and mark C true. I will add new code some other time.

I don't want this

[6950] = {
    ["cn"] = "Systres Tomes and Scrolls",
    ["n"] = "Ode to the Nose of a Woman",
    ["q"] = 6760,
    ["e"] = { --Does not have a location; it is given directly to your inventory in the quest and must be read.
    },
    ["c"] = true,
  },

Having the comments is fine but I don't want to create a big standard for comments either. Just whatever comments works for you. Just keep it simple.

I just want ["e"] to contain the location where it is after the quest like I saw in one of PorygonA's entries.

Sharlikran commented 9 months ago

I found another example to mention.

When you see this, leave it that way. It means it's not updated.

  [1057] = {
    ["c"] = true,
    ["cn"] = "The Five Companions",
    ["n"] = "How Long Before the Echoes Fade?",
    ["q"] = 4607,
    ["e"] = {
      [1] = {
        ["mn"] = 11,
        ["py"] = 0.4329300000,
        ["px"] = 0.7714100000,
        ["pm"] = 7,
      },
      [2] = {
        ["px"] = -0.0969200000,
        ["d"] = true,
        ["py"] = 0.0906600000,
        ["pm"] = 490,
      },
      [3] = {
        ["mn"] = 2,
        ["py"] = 0.3919400000,
        ["px"] = 0.0739500000,
        ["pm"] = 1,
      },
      [4] = {
        ["mn"] = 15,
        ["py"] = 0.7437500000,
        ["px"] = 0.2475700000,
        ["pm"] = 143,
      },
    },
  },

This isn't needed

  [1057] = {
    ["c"] = true,
    ["cn"] = "The Five Companions",
    ["n"] = "How Long Before the Echoes Fade?",
    ["q"] = 4607,
    ["e"] = {
      [1] = { ["mn"] = 11, ["py"] = 0.4329300000, ["px"] = 0.7714100000, ["pm"] = 7, },
      [2] = { ["px"] = -0.0969163978, ["py"] = 0.0906631999, ["pm"] = 490, ["d"] = true, },
      [3] = { ["mn"] = 2, ["py"] = 0.3919400000, ["px"] = 0.0739500000, ["pm"] = 1, },
      [4] = { ["px"] = 0.2475699982, ["py"] = 0.7437764119, ["pm"] = 143, ["qc"] = true, },
    },
  },

I might find a book myself, and go look and if you or someone else already updated it then I will skip it. Because it's not important that the coordinates are different. I could have been standing in a slightly different location. So I'm not going to change other people location. Unless the book moved like you mentioned.

Sharlikran commented 9 months ago

To add some context to this.

[3] = {
        ["mn"] = 2,
        ["py"] = 0.3919400000,
        ["px"] = 0.0739500000,
        ["pm"] = 1,
      },

The books are like that when I saved everything to saved variables from version 23. That's just how the game saves things in a random order. You can't force the game to save that on one line. That's how I know it's older data.

Sharlikran commented 9 months ago

Commit 91286dcb461ca8c379e3caa8ddfdff657061083a is no longer going to be valid compared to master. It's not customary to keep pushing to this branch. I would make a new one.

desertforce commented 9 months ago

Hi Shar,

Thank you very much for your thorouch reply =) !!!

I hope although this is closed, you will still be able to read my answer.

For furture updates/commits coming from me I will see that I split them up. I will try to do as you suggested. One commit is for a city, another one for example for all delves and the public dungeon in a zone and another one for the overland zone. For all commits I will try to limit it to not more than 30 books per commit, if its more, I will split them up into 2x 20 commits (or something like that). In addition to that, if the first 4 digits after the dot (0.xxxx) of the coordinates I collected are identical to the ones already in the code, I will skip them. Unless the entry inside the code is missing something, for example an overland pin or something else. In that case I will update the missing information.

For the commit itself. I will sort it numerical, starting from the lowest number and going to the highest (as it is inside the Eidetic file). As you said, that way you will have it easier to review everything =) .

I forgot with the quests that plop a lore book in your inventory. Do not make some kind of fake entry for them and mark C true. I will add new code some other time.

I don't want this

[6950] = {
    ["cn"] = "Systres Tomes and Scrolls",
    ["n"] = "Ode to the Nose of a Woman",
    ["q"] = 6760,
    ["e"] = { --Does not have a location; it is given directly to your inventory in the quest and must be read.
    },
    ["c"] = true,
  },

Having the comments is fine but I don't want to create a big standard for comments either. Just whatever comments works for you. Just keep it simple.

I just want ["e"] to contain the location where it is after the quest like I saw in one of PorygonA's entries.

I will keep an eye on that. But... Again, just for protocol.

I definitely did not make that entry. I checked all my saved files. The only time I touched that book was the moment I entered the data from BixenteN7Akantor and those included the coordinates it can be found after the quest. Before that I never ever touched it =( ! Also, I always kept in mind one of the first things you Shar teached me in June this year, regarding updating stuff: ONLY make an entry for a lorebook if you do have the right coordinates for it! The only exception are books inside of shelves, but with them you SHOULD NOT bother! For all the books which I found and which are needed to be read from your inventory or can be read at another location I collected the coordinates where they can be found at the begining of the quest and - if possible - after the quest. Those coordinates I entered into the code and added a comment indicating that the book can be read from the inventory or at another location after the quest.

To conclude this matter, I just went through all the updates on Git for this year and found this: https://github.com/ESOUIMods/LoreBooks/pull/12/commits/643b49f933f2a95eec93e040aae521c20acbb5b9 It was PorygonA who entered that "semi-entry" into the code. If he has not read through the comments here, perhaps you should tell him to keep an eye out for this, so that he will not do it again ;-) .

Commit 91286dc is no longer going to be valid compared to master. It's not customary to keep pushing to this branch. I would make a new one.

Yes, will do so. Its something I was planing to do anyways - creating a new sub-branch, using it for a commit and after the commit went through, delete it and create a new sub-branch. Didn't figure out any other way do get clean branches and updates, without always deleting my main-branch (>_<).