Hydra9268 / ZGESO

A public domain ESO Leveling Guide originally produced by Zygor Guides
19 stars 11 forks source link

Review #17

Closed Gator7778 closed 3 years ago

Gator7778 commented 3 years ago

@Hydra9268 here ya go this is my changes and put Sharlikran's fix from an old guide that was misspelled

Hydra9268 commented 3 years ago

Thanks, @Gator7778. I'll review either Thursday or Friday.

Hydra9268 commented 3 years ago

@Gator7778 A few items.

  1. I found and fixed a minor bug.

An extra step caused it.

I updated the parser method to help pinpoint where the problem might exist.

I checked in a set of changes to the Review branch. Feel free to check them out.

  1. I changed the guides' names to Western Skyrim and The Reach so the addon can automatically load the correct guide for new players within the zone.

  2. The Western Skyrim guide doesn't take into account new players. So I'll go ahead and author a small intro guide for that.

  3. Lastly, @Gator7778 @Sharlikran @Krandor1, I added you three to the addon's description and snichols7778 to the author list. So far, the guide looks excellent. 👍

The review continues. 🙂

Sharlikran commented 3 years ago

Initially in https://github.com/Hydra9268/ZGESO/pull/15 I didn't realize the intent of the guide. After realizing that maybe the guide was simply more immersive I asked and Gator confirmed that intended to make it more interesting for users.

I loaded the Addon and did notice the error message about the step, but didn't realize what it meant. I am glad there is a way to point out an error in the mod. That is interesting.

Although I did not know about the step, there were no obvious Lua errors that seemed like a breaking change so I uploaded the changes to ESOUI. Therefore 576 already has the changes. Because of the fixes a new version, 577 will need to be uploaded.

I would like to request that the following is added to the manifest file in the manner mentioned below so that it works properly with Minion.

For the upper portion of ZGESO.txt

## Title: Leveling Guides v1.4.654
## Description: A Zygor addon updated by snichols7778, Hydra9268, Krandor1, for the Greymoor expansion.
## Author: snichols7778, Hydra9268
## Version: 1.4.654
## AddOnVersion: 14654
## APIVersion: 100033 100034
; ## DependsOn: LibGPS AceTimer-3.0>=146
## DependsOn: LibGPS>=65
## SavedVariables: ZGESOSettings
## Disclaimer: This Add-on is not created by, affiliated with or sponsored by ZeniMax Media Inc. or its affiliates. The Elder Scrolls and related logos are registered trademarks or trademarks of ZeniMax Media Inc. in the United States and/or other countries. All rights reserved. This Add-on is not sponsored by Zygor Guides.

Lastly, @Gator7778 @Sharlikran @Krandor1, I added you three to the addon's description and snichols7778 to the author list. So far, the guide looks excellent. +1

I have not done any work except a bug fix for Removing the LibStub requirement is 573 and a minor bugfix in 574. Other then that I simply requested to be added as a maintainer to ensure that the tool would never become non functional in the event that ZOS changed something in the API. If you would please remove me from that list as I have just been keeping an eye on things since your announcement in April of 2020.

image

Especially after seeing that post I felt such a good tool should be maintained.

I am glad to see you are back, ready to maintain the project, oversee its development and, keep it up to date on ESOUI. Therefore I would like to request to be removed as a maintainer. If you decide you no longer have time to maintain the project then feel free to add me again so that I can keep things up to date as needed. I would not have known to make all the changes made to the files in the final draft of the mod. It was interesting to see the changes. Thank you for that Hydra9268.

Thank you and also as mentioned, good work snichols7778 everyone will love the new guides.

Sharlikran commented 3 years ago

One note though in the future according to the UI developer, the checkbox for Load out of date Addons is being removed.

Hydra9268 commented 3 years ago

Although I did not know about the step, there were no obvious Lua errors that seemed like a breaking change so I uploaded the changes to ESOUI. Therefore 576 already has the changes. Because of the fixes a new version, 577 will need to be uploaded.

Sure. Go ahead and make a pull request for a new branch I've created called manual-merge. I'll need to merge your changes into the review branch manually.

I would like to request that the following is added to the manifest file in the manner mentioned below so that it works properly with Minion.

For the upper portion of ZGESO.txt

## Title: Leveling Guides v1.4.654
## Description: A Zygor addon updated by snichols7778, Hydra9268, Krandor1, for the Greymoor expansion.
## Author: snichols7778, Hydra9268
## Version: 1.4.654
## AddOnVersion: 14654
## APIVersion: 100033 100034
; ## DependsOn: LibGPS AceTimer-3.0>=146
## DependsOn: LibGPS>=65
## SavedVariables: ZGESOSettings
## Disclaimer: This Add-on is not created by, affiliated with or sponsored by ZeniMax Media Inc. or its affiliates. The Elder Scrolls and related logos are registered trademarks or trademarks of ZeniMax Media Inc. in the United States and/or other countries. All rights reserved. This Add-on is not sponsored by Zygor Guides.

I've updated the file and checked into the review branch.

Lastly, @Gator7778 @Sharlikran @Krandor1, I added you three to the addon's description and snichols7778 to the author list. So far, the guide looks excellent. +1

I have not done any work except a bug fix for Removing the LibStub requirement is 573 and a minor bugfix in 574. Other then that I simply requested to be added as a maintainer to ensure that the tool would never become non functional in the event that ZOS changed something in the API. If you would please remove me from that list as I have just been keeping an eye on things since your announcement in April of 2020.

I added you and @Krandor1 to the description because you both helped update and maintain. However, I didn't add you both to the author because neither created a guide. I think that's an excellent solution for appropriate credits.

image

Especially after seeing that post I felt such a good tool should be maintained.

Thank you! 🙂

I am glad to see you are back, ready to maintain the project, oversee its development and, keep it up to date on ESOUI. Therefore I would like to request to be removed as a maintainer. If you decide you no longer have time to maintain the project then feel free to add me again so that I can keep things up to date as needed.

Technically, I'm not "back." I wanted to help polish the guides. I would greatly appreciate it if you could stay on to help maintain.

That said, I may clean up the addon from time to time (but I make no promises to do so).

Thank you and also as mentioned, good work snichols7778 everyone will love the new guides.

Indeed! Thank you, @Gator7778 !

One note though in the future according to the UI developer, the checkbox for Load out of date Addons is being removed.

Could you please link to the announcement?

Sharlikran commented 3 years ago

Could you please link to the announcement?

I didn't make a big post like you did it was just a comment.

Technically, I'm not "back." I wanted to help polish the guides. I would greatly appreciate it if you could stay on to help maintain.

Okay, then rescind the request. However, I could tell that I don't know the program well enough at all. I have a lot to learn as it is. It's a daily learning experience.

Hydra9268 commented 3 years ago

Could you please link to the announcement?

I didn't make a big post like you did it was just a comment.

I want to review what the UI developer said about the "checkbox for Load out of date Addons being removed." That seems like a big deal, and if it is true, it would be a terrible move on Bethesda's part. However, I'd like to read what the UI developer wrote for context.

Technically, I'm not "back." I wanted to help polish the guides. I would greatly appreciate it if you could stay on to help maintain.

Okay, then rescind the request. However, I could tell that I don't know the program well enough at all. I have a lot to learn as it is. It's a daily learning experience.

When I worked on the first version of the Summerset guide, I asked Zygor questions about their addon but never got a response, so I had to stumble through it until I figured it out.

I WON'T do that for people like you who actively maintain the addon or @Gator7778, who created a guide. If you ask questions, I will try to assist. 🙂

Sharlikran commented 3 years ago

Oh I get it, okay so no, it's not like that. I don't know the UI developer personally. Like we don't hang out and have a barbecue on the weekends.

As far as this project goes I just don't want to discuss this particular topic here.

It's really kind of like how I work with the Bethesda Community manager so if he tells me something I just know it because he told me.

Gator7778 commented 3 years ago

Could you please link to the announcement?

I didn't make a big post like you did it was just a comment.

I want to review what the UI developer said about the "checkbox for Load out of date Addons being removed." That seems like a big deal, and if it is true, it would be a terrible move on Bethesda's part. However, I'd like to read what the UI developer wrote for context.

Technically, I'm not "back." I wanted to help polish the guides. I would greatly appreciate it if you could stay on to help maintain.

Okay, then rescind the request. However, I could tell that I don't know the program well enough at all. I have a lot to learn as it is. It's a daily learning experience.

When I worked on the first version of the Summerset guide, I asked Zygor questions about their addon but never got a response, so I had to stumble through it until I figured it out.

I WON'T do that for people like you who actively maintain the addon or @Gator7778, who created a guide. If you ask questions, I will try to assist. 🙂

yea i did alot of stumbling in the beginning i tried asking zygor as well they said sorry we cant help you ask the community

Hydra9268 commented 3 years ago

Oh I get it, okay so no, it's not like that. I don't know the UI developer personally. Like we don't hang out and have a barbecue on the weekends.

Oh, I see. So I should take the information with a healthy bit of skepticism. ESO has supported the loading of older addons since it was released, so to suddenly remove the capability would be strange. That said, since I reinstalled, I've had to delete a couple of addons because they don't work even with the outdated API version. The code in the addons points to code that appears not to be available.

As far as this project goes I just don't want to discuss this particular topic here.

It's really kind of like how I work with the Bethesda Community manager so if he tells me something I just know it because he told me.

Even after I finish reviewing the new guides, you won't be alone in maintaining the addon. You'll continue to have my support where I can provide it.

Gator7778 commented 3 years ago

now that im more fluent with the coding ill be fixing alot of errors that have accumulated in the older guides

Hydra9268 commented 3 years ago

I WON'T do that for people like you who actively maintain the addon or @Gator7778, who created a guide. If you ask questions, I will try to assist. 🙂

yea i did alot of stumbling in the beginning i tried asking zygor as well they said sorry we cant help you ask the community

Yup. It's a business for Zygor. So it is understandable that they won't spend time assisting a not-for-profit addon. We haven't seen it that way. We see it as a valuable tool worth preserving for the ESO community in our limited capacity.

Hydra9268 commented 3 years ago

now that im more fluent with the coding ill be fixing alot of errors that have accumulated in the older guides

🙂👍

Gator7778 commented 3 years ago

so how did i do on the guides

Hydra9268 commented 3 years ago

Although I did not know about the step, there were no obvious Lua errors that seemed like a breaking change so I uploaded the changes to ESOUI. Therefore 576 already has the changes. Because of the fixes a new version, 577 will need to be uploaded.

Sure. Go ahead and make a pull request for a new branch I've created called manual-merge. I'll need to merge your changes into the review branch manually.

@Gator7778 @Sharlikran If you have minor edits you'd like to get into the review branch, please make a pull request for the manual-merge branch. I'll manually merge them into the review branch.

Hydra9268 commented 3 years ago

@Gator7778

so how did i do on the guides

So far, it looks great. You're using commands even I didn't use in my guides, demonstrating an excellent grasp of the addon's language.

I'm still in the process of working through them.

Gator7778 commented 3 years ago

@Gator7778

so how did i do on the guides

So far, it looks great. You're using commands even I didn't use in my guides, demonstrating an excellent grasp of the addon's language.

I'm still in the process of working through them.

I learned as i went through by the 2nd time through i had it mostly figured out and if you mispell something it will let you know real quick

Gator7778 commented 3 years ago

@Sharlikran dont worry as long as playing the game the guides will come out before or shortly after launch i just need quest map and LibQuestData to be upto date so i know where all the quests are before i start working on it. Oh shit your one of the admins on both good deal we can work together to make sure guides come out before launch.

Sharlikran commented 3 years ago

@Gator7778 I don't always have time to do that prior to release. It could take time and I depend heavily on other users as well. However, I get your point and I understand that it makes it easier.

Gator7778 commented 3 years ago

either way it wont be at the end of the dlc this time lol

Hydra9268 commented 3 years ago

@Gator7778 What are you using to get current X and Y coordinates?

Hydra9268 commented 3 years ago

@Gator7778 Actually I figured it out.