Lightman's Currency

Lightman's Currency

2M Downloads

Feature Request: Changes to the Network Terminal Screen

sesgoe opened this issue · 7 comments

commented

Is your feature request related to a problem? Please describe.
It's annoying when I open the Network Terminal Screen, and there are useless traders for me to click on. Some traders are completely empty (maybe they placed down the network terminal and forgot about it), and some traders set up some initial stock, and then stopped playing the server completely. Life happens! I'm not really complaining about the empty traders existing -- I'm complaining that I have no way to hide them on the Network Terminal UI.

The other issue I keep bumping into is that when I search for something on the Network Terminal screen, if I click into a trader, and click back out, that search text gets wiped out, so my workaround has been to keep it copied on the system clipboard so I can continually paste it back in as I'm hunting for whatever I'm looking for. Having a small bit of persistence (doesn't need to be more than during the duration of Minecraft being open) would be super helpful here.

Describe the solution you'd like
I made a small effort in this PR here: #227 that details what I was looking for. The only thing I considered adding but ultimately didn't was a "clear" button for the search prompt.

Here's the tiny 10-second clip from the PR detailing the changes:

small_network_terminal_demo.mp4

Describe alternatives you've considered
There isn't really an alternative solution for my first problem, and my second problem can sort of be handled by copy-pasting, but it is not ideal, and annoying in general.

Additional Context
Apologies if I offended by way of making a PR -- I certainly didn't mean to imply you couldn't make the change yourself. I have this implemented for myself client-sided on the server I play on, so for me personally, it's not a huge deal whether you implement it or not, but I pinged my PR out to the server community and got some support, so it seemed like making this a public change would be in the interest of a bunch of players.

We use your mod's traders + currency pretty heavily over in the https://www.curseforge.com/minecraft/modpacks/craft-to-exile-2 modpack community, and it's the single best trading experience I've ever had as a Minecraft player. So if nothing else, please know you've built a really fantastic mod that handles all kinds of edge cases, and it's an amazing piece of work. Thanks for building it.

commented

I do plan on adding more filtering options to the terminal at some point, so the "hide empty traders" potion of this will probably be included with them. Though one option I've considered that would probably make things easier in the meantime is perhaps change the text color for the traders name depending on it's trade state (red for no trades at all, orange for all trades out of stock, white for "normal", etc.).

The "remember the last search" feature should also be fairly easy to implement in the same way that I have it set up to remember your scroll state (which it already does, and was a very necessary thing back before it took up the entire screen), so I'll make sure this at the very least gets added in the next update.

On another topic, I'm not actually upset or offended about the pull request, my main issue with the PR was more that things were coded in a somewhat sloppy way (well I say sloppy, but that could simply just be a difference in our coding methods and/or your ignorance of how my system is designed as a whole, etc.) that broke other compatibility features and more or less hard-coded it to only work with Item Traders. I like to keep my mod open to add-ons, which means that most things should be left to the traders discretion on how to handle them (which is why theITraderSearchFilter class exists, so that each trader can manually filter any custom added trader types to search results as required).

That and while I don't mind people looking through my code searching for potential issues/optimizations/suggestions, or even posting edited code as a suggestion, while this is an open-source project it's not necessarily an open-contribution project. Unfortunately Github doesn't have the ability to simply disable Pull Requests on a repository, otherwise I would already done so, as I personally don't see a reason to have them on a single-person project.

commented

yeah on big servers this is kinda necessary cuz players straight up have to go through every single shop one by one to figure out which ones are staight up empty or have no stock. needs that qol. also pr looked cool

commented

done so, as I personally don't see a reason to have them on a single-person project.

No worries! Yea, this is a long standing issue with github in general. My apologies for any annoyance caused. It might be worth sticking a note in your README or something that you generally don't accept feature PRs. I saw that there was a merge from another PR (granted, it was translation related) so I figured another PR was fair game.

Like I mentioned, this is my first foray into modding at all, so I expected the code to be sloppy/bad. PRs in my mind are a vehicle to start a good faith discussion about something new, and it seems like it accomplished that goal here! In the best case I learn something about how I should have written it, and a few of your comments here taught me a few new things, so thank you!

Thanks for considering the change, and I look forward to the screen reworks!

commented

Trader colors now change based on trade count/stock count in v2.2.2.0.
I'll work on adding the additional filter options in the next update.

commented

I do plan on adding more filtering options to the terminal at some point

Maybe you could use GitHub like flitering (you will have to use your own filtering options but the system will be intuitive to users)?

Image

commented

Maybe you could use GitHub like flitering (you will have to use your own filtering options but the system will be intuitive to users)?

I do like this idea, partially because it won't require adding new widgets to the screen, and because it'll be significantly easier to implement, though it likely won't be able to have auto-fill suggestions like github does

commented

I know it's been a long-time coming and it's not exactly what was originally asked for, but as of v2.2.4.3 you can filter traders from the search bar (as well as from the /lcadmin traderdata search <filter> command since they use the same system).

A few example filters:

  • type:item will limit the results to only item traders
    • Partial searches are allowed & encour, including searching by namespace so you can search type:lctech to only see fluid/energy traders, etc.)
  • owner:lightman will limit the results based on traders with a matching owner
  • name:example will limit the results base on the traders current display name
  • item:stick will limit the results based on whether the trader has a trade selling and/or buying a stick

All of the above filters support partial searches (i.e. searching for type:ite will still show item traders, it just might also show some fictional netherite trader type if such a thing existed).
The filter itself is also optional and for the most part they simply let uou limit the search to that particular topic (i.e. searching for item:stick will not show traders that are owned by a player with the name of "stickman" that would otherwise also appear in the search if you had simply typed stick in the search bar, etc.)

Some extra filters directly compare numbers and can be input in several ways:

  • trades>0 will limit the results to traders that have more than 0 trades.
    • Number fields also accept <, >= and <= when comparing against single numbers
  • id=5 will limit the results to the trader with the internal trader id of 5 (also accepts id==5 as I keep typing == by force of habit as that's what's done in code for check equals)
  • stock[5,10) will limit the results to traders with 5 or more trades that are in stock but also less than 10 trades in stock.
    • As is normal for range fields of this format, either end of the brackets can be square [] or round () where square is inclusive and round is exclusive.

There is also a single boolean filter

  • ready:true will limit the results to traders that are considered "ready for customers", and is also a default filter defined in the client configs network terminal section.
    • You can also do ready:false to explicitly look for traders that aren't ready as well, because why not?
    • Fun fact: I originally intended for the default filter to be trades>0, however this had the downside of hiding the Auction House from the terminal if nobody had made an auction yet, and naturally that is a no-go, so instead we got this specialized filter :)