manavortex / FurnitureCatalogue

FurnitureCatalogue
4 stars 9 forks source link

Discussions here #59

Open wookiefriseur opened 1 year ago

wookiefriseur commented 1 year ago

We can use this issue if we want to discuss something. That way we don't have to use PRs for that.

ToDo: Setup discussions

wookiefriseur commented 1 year ago

@BerylBones Were you planning on doing luxury furnishings this weekend? I wanted to have a look at the PTS but I could also do the luxury furnishings while I'm at it. Either way is fine for me. Let me know

Edit: done and prepared a bit for the next patch #76

BerylBones commented 1 year ago

@wookiefriseur Sorry, busy weekend!

I see you pulled in all the new items as well, thanks for doing that! I'll try to add known sourcing for the new things within a week or so of update 39 launching. Most of them I think are crown items so should be a fairly quick update.

wookiefriseur commented 1 year ago

Nice, I was not sure how to best get the sources.

Added some stuff and prepared for the next update but I'll wait with the next release until the patch is out.

In the meantime I'll keep trying to fix issues and maybe have a jab at search/load performance.

wookiefriseur commented 1 year ago

@BerylBones I hope my recent changes didn't make it harder for you to contribute. Let me know if you need help with anything. I'm thinking about making a webinterface for the database, so it'll become easier to maintain.

BerylBones commented 10 months ago

@wookiefriseur I am so sorry that I disappeared on you. I've had one crisis after another and I should have reached out to let you know I'd be gone for a while.

But, I am back now and ready to get this updated. I've pulled your latest update from the dev branch and will be working on it today. Let me know if anything else has changed!

wookiefriseur commented 10 months ago

No worries, good to have you back.

I did not notice many things missing ingame so it was low priority for me too. Mostly just did luxury furnishing updates and some missing items. But it's perfect timing, now that the new update just dropped.

So far I only prepared version updates for it and the grandmaster crafting stations. Will push it in a minute.

Edit: Pushed #90 , let me know if you need anything.

BerylBones commented 10 months ago

I'm almost ready to push this update, but I'm not sure of the best way to handle the new achievement furnishings. They are purchased with archival fortunes instead of gold. For now I can use the existing setup, but let me know if you think of a way to note the different currency.

manavortex commented 10 months ago

Back in the day, I handled that by shoving it into a different table (for the IC currency). By now, it might be easier to give them a currency flag?

wookiefriseur commented 10 months ago

Yeah, will probably use the simple text for now, because currently there are only a handful of items.

But it seems like there might be a weekly rotation for one of the Endless Archive traders, so maybe I'll make a separate file later (if it is similar to the luxury furnishings)

BerylBones commented 10 months ago

The merchant with the rotating inventory shouldn't have anything that's exclusive to him. Everything he sells you can get as a drop somewhere, so I wouldn't worry about setting anything up for him.

manavortex commented 10 months ago

In that case, why not have a list with only item IDs that this guy can possibly say, and then concat or sometimes sold by <this one dude> to the tooltip if it’s in there?

On 4 Nov 2023, at 16:50, BerylBones wrote:

The merchant with the rotating inventory shouldn't have anything that's exclusive to him. Everything he sells you can get as a drop somewhere, so I wouldn't worry about setting anything up for him.

-- Reply to this email directly or view it on GitHub: https://github.com/manavortex/FurnitureCatalogue/issues/59#issuecomment-1793481201 You are receiving this because you commented.

Message ID: @.***>

BerylBones commented 10 months ago

Yes, we can definitely do that. I did not for this update, but I've made a note of which items he has this week so I can add it in for next time.

BerylBones commented 3 months ago

I am apparently very behind on updates, so thank you so much @wookiefriseur for keeping this going. I didn't realize how far behind I was until I tried to push Gold Road items today and barely recognize the addon lol. I think I got it mostly figured out (minus the actual publishing), but please let me know if I messed anything up. I'm hoping to dig into the rumor item sources this week while I have some extra time.

wookiefriseur commented 3 months ago

Ohh, thanks for the update @BerylBones !

I'm in the middle of overhauling the translation system. That's why the current dev branch might look a bit messy. Good job figuring it out though, that makes 1 of us. 😄 Usually this would work without problems and dev is always in a state that can be published. This time however an emergency came up and now it's in a half done state. I'm not even sure if shows all translations.

I'll be done within the next 2 days. Don't worry about any merge conflicts or publishing. I'll take care of that. You can continue with your current code version.

If you're interested in the publishing process I can guide you through it so you can publish whenever you like next time.

Also thanks a lot for sourcing the items. I don't have Gold Road so it would take me longer to do it.

BerylBones commented 3 months ago

I'm a quick study! 😁 When you have time, a walkthrough the publishing process would be great! No rush through, and I'll work on item sourcing in the meantime!

wookiefriseur commented 3 months ago

Alright. I updated CONTRIBUTING. You can see the general flow in the diagram, maybe it helps.

I should have time to walk you through a release later this week. Won't take long. More than enough potential release dates to pick from. And there is a luxury furnishing update every week, so even that would work as a release 😃 (that smiley looks a bit unhinged, when you zoom all the way in)

wookiefriseur commented 3 months ago

The dev branch is now up to date with the master and on on top of it are your recipe-PR #141 and my string overhaul PR #142

I wasn't able to test the English client yet, so maybe some strings still look strange there. But all in all this is ready for release. You can check the dev branch out and do your item sourcing on that version (if you haven't already started).

Let me know if you run into any problems

Edit: woops, the fast forward merge duplicated strings and deleted the Gold Road string. force pushed a fix. now it's really up to date.

Edit2: Had time to check the English client. Fixed any major localisation issues. We're "international" now. 😁

wookiefriseur commented 2 months ago

Ohh, @BerylBones I just remembered that you probably don't have merge permission on this repo. We used to work on your fork, then you made pull requests, then manavortex merged it. I only got full merge permission, because I asked to be maintainer.

So there is not much to walk you through right now regarding publishing, unless you have permission or get permission. But nevertheless just let me know ingame or on Gitter/Element/Matrix if you need help with anything, for instance with git or the code. I can also share my screen.


The whole process for you right now is like this:

  1. pull the latest dev branch from the main repo (here)
  2. make some changes
  3. pull from time to time, in case dev on the main repo has changed
    • git pull https://github.com/manavortex/FurnitureCatalogue.git dev
    • or git pull --rebase https://github.com/manavortex/FurnitureCatalogue.git dev
  4. when you're done make a PR to dev on the main repo (you can just delete the prefilled PR-text and make your own)

After that comes my part:

  1. I merge your PR with the other stuff
  2. then I make a new PR from dev to master, both on the main repo
  3. I copy any CHANGELOG relevant notes from your PR
  4. I set labels for the release, so the bot knows what to do
  5. then I merge on master

And now the automated parts:

  1. the bot is triggered, updates the AddOn version and generates some files for us
  2. the bot creates a PR from the branch release/1.2.3 to master with those changes (like #131 )
  3. I check if everything looks fine and then I merge it
  4. this triggers a different bot action and it creates a release here
  5. the bot then publishes the release on ESOUI
manavortex commented 2 months ago

@BerylBones added you as collaborator :)

BerylBones commented 2 months ago

Perfect, thank you both! I have a couple more item sources I want to add in today, and then we'll give this a shot!

BerylBones commented 2 months ago

Ok, I think I missed a step somewhere. I managed to get everything merged onto the master (set label as 'release'), but it doesn't look like it triggered the automated process.

wookiefriseur commented 2 months ago

Oh yeah, I left a comment in #144 . The automatic script tried to format the files before creating the release but then encountered a syntax error. We can either roll it back or do a small patch-PR from dev to master and try again.

Other than that the steps looked all good 👍

BerylBones commented 2 months ago

Ok, trying it with a small patch-PR

wookiefriseur commented 2 months ago

Ok, we're getting closer. 😁 The script said: error: could not format file ./data/MiscItemSources.lua: error parsing: error occurred while creating ast: unexpected tokenstrGeneric. (starting from line 91, character 30 and ending on line 91, character 40)

I don't see the error in GitHub right now, but i'll try locally

Probably doesn't like that it's on multiple lines

BerylBones commented 2 months ago

I think I found it. Missing comma in the vvardenfell painting sourcing

wookiefriseur commented 2 months ago

Ok, you know the drill. I hope it works this time. But so far it's good that the script caught those.

I kept looking locally and was confused but then I noticed that I was looking at an old version 😁

Edit: Yeah, you did it!

BerylBones commented 2 months ago

Woo! Third time's the charm!

wookiefriseur commented 2 months ago

So glad it worked. My palms were sweaty all the time because I had not tested the new bot script yet. 😁

The release should show up on ESOUI soon.

Good job 🥳

BerylBones commented 2 months ago

Looks like everything is working! Thanks for walking me through everything.

wookiefriseur commented 2 months ago

I prepared the luxury furnishings for this weekend but hadn't time to add them yet. And it looks like you already added 1 of this weekend's luxury furnishings.

If you want to practice some more, you can also do the rest tomorrow and make another release, this time with a MINOR label.

Here, if this helps (in German, but the IDs are there):

    [134465] = {        -- Varlastein, glühend
        itemPrice = 5000,       -- Gold
    },
    [134466] = {        -- Ayleïdenhalterung, leer
        itemPrice = 4000,       -- Gold
    },
    [134467] = {        -- Culandastein, glühend
        itemPrice = 5000,       -- Gold
    },
    [134468] = {        -- ayleïdischer Schalter, uralt
        itemPrice = 4000,       -- Gold
    },
    [156664] = {        -- Ayleïdensäule, klein und leer
        itemPrice = 5000,       -- Gold
    },
    [171421] = {        -- Ayleïdensäule, groß und leer
        itemPrice = 14000,      -- Gold
    },
    [182629] = {        -- ayleïdische Feuerschale, Stein
        itemPrice = 10000,      -- Gold
    },
    [193801] = {        -- Ayleïden-Trennwand, gewölbt
        itemPrice = 20000,      -- Gold
    },
    [203582] = {        -- ayleïdischer Durchgang, hoch und gewölbt
        itemPrice = 20000,      -- Gold
    },

If not, I'll just do it on monday or so

BerylBones commented 2 months ago

Oh right, I forgot about changing the date on the older lux items. I can do that, and I need to fix the sourcing for gold road pickpocket items.

wookiefriseur commented 2 months ago

Alright, thanks! Don't forget the MINOR label. That should update it to 4.82.1

I also edited the script to actually save all lua files changed by the formatter (that part was missing, woops 😊 ).

Oh, yeah I see the pickpocketing:

-- generic formatter is like :
strGeneric(PREFIX, ADDITIONALINFO, SOURCTYPE, SOURCE)

 -- that should do it for pickpocket in Westworld in general:
local pickpocket_weald = strGeneric(srcPick, nil, "loc", loc.WEALD)

-- if it's from any specific npc types in Westworld, you could do:
local nob_thieves = strSrc("other", npc.CLASS_NOBLE, npc.CLASS_THIEF)
local pickpocket_weald_nob_thieves = strGeneric(srcPick, nob_thieves, "loc", loc.WEALD)
wookiefriseur commented 2 months ago

Well done. 1st try. I should have said PATCH instead of MINOR. Got it mixed up, but not important 😄

BerylBones commented 2 months ago

@wookiefriseur did you contribute at all to ESO-Hub's new furniture catalogue tool? I've already received enough questions about it that I'm about to email them to ask them to change its name, but I don't want to do that if you're part of the project. 🙃

wookiefriseur commented 2 months ago

@BerylBones No, first time I'm hearing of this. I wouldn't do anything major without talking about it first.

Looks like what MMO Fashion used to do, but for furniture. Yeah, that name can cause some confusion. I'm currently sick so I can't think up a good alternative name. 🤧