-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Trending Builds section should be not be present. #7944
Comments
Well it used to load, now it is not loading. So, wish granted ? |
It is worth noting too that this site does not ask build creators for permission or consent to upload their builds. As you can see on any build page, it doesn't ask permission and just lets build creators claim their stuff afterwards, requiring a path of exile account, reddit, or patreon login to do so. I do not think a site which doesn't even ask consent or permission for its content should be a core feature for Path of Building. |
Any builds that are there because of Patreon should have a giant ass border around them saying "SPONSORED" |
I've opened a PR to address this issue. In addition to the concerns above, adding a dependency to 3rd parties not affiliated with GGG is probably unwise from a development perspective. The trending feature is already not working properly because the site is essentially being ddos'd by everyone using POB. |
I think the feature itself is good but linking specifically to POB Archives is the problem. There should be an open api spec that works with POB that any website could implement to get builds to show up. Then there should be an option to add websites to POB to pull the builds. I can imagine the dropdown box where PoB Archive is listed having several sites listed. Maybe a manage button next to it to add sites with the default being no sites. That way if you want to still use PoB Archives despite the problematic issues with the site you could add it. |
This exists. PoB Archives implements this class to be in that dropdown: https://github.com/PathOfBuildingCommunity/PathOfBuilding/blob/dev/src/Classes/ExtBuildListProvider.lua Other websites can do the same, similar to how the website dropdown works on the Import tab |
That is the inverse. Now API maintainers would have to contribute to PoB, and maintain the PoB implementation. With the proposed API spec, PoB would not change, and users would just add an API to the list to pull from (and the other ends maintainers would never have to touch PoB). Apart from that, isn't pulling data from external APIs kind of spooky in any case, whatever the implementation, you never know what kind of nasty things people start pushing to the PoB userbase, like say, someone purchasing an advertisement in an implemented website to promote their discounted OF, or just random hate speech? That brings a moderation requirement for PoB. (Especially if the sites are implemented in PoB, i.e. part of the release, vs. users opting in by adding the sites manually as they wish, to see whatever they decide). |
Adding a class to PoB isn't an OpenAPI spec. I am thinking of an API that returns json that is then consumed by PoB to create the build list. So a json schema and set of standard apis need to be documented then a generic scrapper that uses those to pull data |
This should really be opt in rather than on by default and should be user driven and not something added by default like with the archive situation. User driven as in users specifically adding a site or what have you they want to get build resources from there rather than having preset unrelated profit driven sites immediately shown or linked without the user wanting to engage with said sites or be linked to them. The similar builds button also should not be on by default. |
I agree that is why users would have to add the sites manually. Then the responsibility is on them for verifying the external site to be legit. Also if done correctly scrapping json shouldn't really be spooky or anything. |
The issue is not whether it's technically possible or not but rather that the current default is a single website that no one opted into and is a for profit closed source tool. Nothing wrong with listing PobArchives as a possible source but never as the very first thing users see when they open up the tool. |
If it should stay there probably should be a change in the API as in a PoB setting that will have a list of possible endpoints for various sources, which may be used as source. |
Okay but what kind of endpoints you can have for a data source like this? You cant source poe.ninja because of how random bullshit go that site is without filtering. Besides PoE archives, there is no specific curated build list tailored for this thing. forum is dead too. feature should be gone entirely. |
@caner-cetin I edited your post as there is no room for personal attacks here. |
oops. |
also, no one mentioned it yet, related PR #7951 |
That's why there should be consideration in rewriting the API, as in for example PoB says: You have a baseurl like: somepoebuildssite.com And PoB would support endpoints such as: filtering specific stuff such as gems etc.: ${baseurl}/v1/builds?query=... Also going from a I think it's either OpenID or OAuth specifiction the .well-known path option a site could implement a standardized path such as .well-known/pathofbuilding and list endpoints for various actions such as querying, listing, statistics etc. whatever PoB would consider supporting. But this is only one of many possible ways but it would at least go into the direction that there is no need for active contribution from build sites into PoB and PoB having to manage and curate those sites, since if the user can just add arbitrary URLs for various services it would be the user's responsibility to add those. Edit: What I believe I forgot to mention or what might not be clear that PoB will describe in what kind of format the returned data should be in, such as a JSON with fields such as buildString, name, externalUrl and whatever else might be necessary. |
Dont go harsh on people. Chill. This feature’s PR was open for 5 months, nobody except Wire said anything, now you cant just come and shitstorm the place. Give whatever your constructive criticism is and leave. okay everyone knows situation is bad, you cant go artillery on contributors with saying “situation is bad”. you are adding nothing to the discussion |
Okay but as I said, we, and most importantly contributors know its a legitimate problem and a serious mistake. Main problem is you are adding nothing to the discussion by reiterating same things, and going ham on these people. |
Feel free to stop using PoB...? No one is forcing you to use it if you don't trust the maintainers. |
This is a pretty childish response though. It was a PR up for 5 months with the only interaction there being Wires doing basic UI and implementation checks with no other user response. The intent behind this addition wasn't super clear and as such got merged, is odd it didn't get put on the beta branch first but mistakes happen. Nothing dangerous was put into the program. The people working on PoB aren't making profit so I don't think your personal trust loss matter as most people can see it was just an oversight. |
As potentially useful as such a feature could be, the current implementation exposes the project to unnecessary risk and maintenance. The current set up establishes the project as curator of approved websites which acts as a type of endorsement especially whatever the default is. The current controversy displays some the risk, but it could be much worse. After approval and integration the site could change the content to hate speech or direct urls to malware, phishing, etc. Having users add sites themselves would largely mitigate this. The current setup also burdens the project with maintaining an implementation for each site. This makes unnecessary extra work for maintainers that should be the burden of the websites themselves. A more generic approach and an api spec would greatly reduce the maintenance required. As is I would recommend removing it and either dropping it or refactoring. |
Observations: The current reddit post and creator coverage is creating (and going to continue to create) a significant influx of people who have no idea what they're talking about and acting entirely by reaction. Apparently it's difficult to separate the feature from the site it's linked to. Opinions: The feature is a great addition. Having additional sites implement this would grow the feature. That's outside of PoB's control. Suggestions: For now, disable this feature by default under a category such as "Third-Party Integrations"; add an option to disable the "Similar Builds" button in the same category. On the longer term, implement the suggestion posed by @gaschenk in that the "similar builds", "recent builds", and "trending/featured/popular builds" have a contract that PoB expects an api to be located at and return, and allow users to add sites by means of adding the site to a list, e.g. http://api.myscummybuildaggregator.com and make the displaying of such builds standardized. |
nothing needs to be thrown away to be fair If that section is going to link to anything, it should link directly to the guide. Not a site that contains a link to that guide. In fact it should be just a "mod". A separate add-on that @canuyal can maintain for himself. As it stands it's literally just bloat, and turns PoB into platform and not an extremely fancy calculator. |
Most people commenting here are already aware of that since it's not hidden at all. As was mentioned the PR related to the feature was up for 5 months without anyone voicing concerns about it or the intent the creator had making it and it got merged with the most likely the positives of the potential core system and not what it actually was being considered. They did skip the beta branch unfortunately as from what I know now it's barely utilized. There's ways a system like this can be positively utilized while difficult though the current rendition is less than desirable, I'm guessing it'll be walked back/removed and looked more into due to the current backlash and discussion around the addition. |
As a user, I don't scan the pull requests of PoB for potential new features to flag, because I'm not a developer, and they've done a good job of adding new useful features without my input so far. The lack of comments on the PR isn't the positive you think it is - it indicates the dev community isn't engaging with the design intent of the PR. Currently, POB is (and is described as) a offline build planner. And a dev just merged a feature which made it non-offline by having a API pull on the opening page, without any consideration of "is this the direction we want to take POB" "is the website pulling down relevant builds" "Have we load tested the site so this feature works against league-start load" "What's the risk of build titles saying gross shit" - a bunch of design considerations that should be thought about before the PR was merged to main. I'm not going to pile on the bandwagon against the dev here - I think both @canuysal and @Wires77 were genuinely trying to add what they thought were useful features. But this entire process has showed laxness in the development process by treating it the same as the many bugfix or small mechanical improvement PRs that have made POB what it is today. |
The problems you're outlining are not unique to this repository and are endemic of open source projects as a whole. If you would like to be part of the solution, please review pull requests and help to steer development of projects you enjoy. This conversation does nothing but discourage the current contributors from continuing. |
This was the response I got the last time I criticized over here. (Involving a PR blindly nerfing damage with the evidence supporting not removing it, and years later that damage being added back) Maybe yall would get more help if you stopped acting like open source is a cesspool and generalizing that all open source projects are like this. I can find plenty over in the KSP world that have plenty of engagement by their contributors, including notes on things discussed in Discord. |
I personally disagree with the intent of @canuysal, it seems very sus within the given context as PoB Archives was only introduced on Reddit 7 months ago, so to me it looks like an attempt to nickel and dime through patreon but that's subjective. I think @Wires77 is simply trying to fulfill his role in the PoB community. I echo your sentiment regarding reviewing pull requests, I only came on here because I feel that an open source project should not enable profiteering. The idea is a good one, but the functionality is bad and given the complexities of verifying whether a build is good or not, I'd say it's not possible to implement a fully autonomous build website which showcases good builds by simply scraping anything that lands in the path of building subreddit. |
I think the point is that such a deviation in the project should have a wider discussion. Possibly even have a discussion on Reddit (not sure where else is better) with the greater community. I'm a software developer, I've considered contributing to PoB but even if I started (and a few others like me) that doesn't mean the next controversial feature to come along is going to be caught. It's not like these happen every day so what you are asking is for people to sit and watch every PR for years until they finally get a chance to give an opinion on a controversial feature request. It shouldn't be a requirement to be a regular contributor to have some heads up that something like this is going to be merged. Other projects such as Godot have community polls to cover such use-cases. Imho, if someone is discouraged from contributing because they might start drama in the community they need to serious consider the kinds of changes they want to make to the project. No reasonable person is going to be discouraged from fixing bugs after seeing this situation. |
@Bellabong Alright, point of clarification: I'm not an active contributor, I'm just trying to cut down on as much reactionary attacking of the people who are as possible. I'm not saying open source is a cesspool. I'm saying that it's not fair to hang the contributors out to dry for not speaking on something that was out of their wheelhouse. @Rybadour Unless I misunderstood what was being said, which is always possible, there were 5 months with which anyone from anywhere could have commented on the addition of this feature. It wasn't sneaky. It wasn't obfuscated. It was done in plain day, and no one spoke up against it. I'm getting wrapped on the wrong side of what I said here, or at least what I intended. There is a subset of people that, when they see drama, they cease to contribute. To reiterate: it is unfair to blame the whole of contributors for not reviewing a feature addition that is modifying something they don't regularly interact with, and doing so will likely put a sour taste in their mouth. That is all. I've said my piece. I'll stop trying to defend and/or speaking for people now. Apologies. |
I think it's hard to say, as someone who is new to PoE and PoB, how do I comment on a PR with no experience in coding on what I believe is the correct way forward on PoB. Initially when PoB was updated I assumed that PoB Archives was PoB pulling commonly imported PoBs and uploading them in some way. I do not know how the system works, I do not know the proper etiquette on informing of PRs I do not want to see. PoB's about section is Offline build planner for Path of Exile. The very first word being Offline, the implementation of an online trending/latest build section seems counterintuitive. Something like this should of first gone through a Beta branch, and if it was how would I an average end user discuss displeasure with the new PR? Is there a Discord I can go-to to discuss the beta branch? Somewhere I can easily discuss with people whom clearly are more knowledgeable on how the system works? The PR being available for 5 months is irrelevant if the average user base is not reading PRs since they don't know what a PR is. I don't know, the Trending part seems like a weird way to try and integrate an online system to an offline builder. I don't know how Icy Veins, Poe Vault, etc would get their own links and trending spots in PoB. Would each site that wants to be added have to put in a PR? Does someone get the left side to equalize the builder? |
I commented on the revert PR but I'll add my 2c in on this issue as well... Path of Building is amazing software, and it does it's job - build calculation - fantastically. Adding the amalgamation of public builds and a browsing feature feels like a significant step away from it's original purpose. Sometimes, in my opinion, it's much better for a tool to do one job, and do it exceedingly well, rather than scope creep out into different areas. Often these secondary features are only half baked, and add significant maintenance overhead to the project. It really feels like separate websites, whoever they are (maxroll, pob archives, poe vault, poe ninja) are a more suitable place to take care of this amalgamation problem. Please don't go down the "Path of Building has to be the only needed tool" path. With that said, I acknowledge that nothing was said on the original PR so I don't feel like this is the merger's fault in any way. |
@MstrGrth I think you'll largely find answers above but I think I can summarize again since this is lengthy.
Opinion: You are an outlier and should do your due diligence to research tools you pull off of the internet. Unfortunately it was poorly timed and coincided with some drama.
Counterpoint: There are currently market search features and a self update feature that both use internet connectivity, so I'd argue the definition of "offline" at this point is more "desktop-use-designed."
I don't disagree with this at all - it should have gone through beta, but it was missed and now we are where we are. Mistakes happen. We're all human. Other PRs DID go through beta, as I was actively using the beta all week and can personally vouch. For clarification, you can opt in to the beta in the bottom-left Options button.
You're in the right place. Open an issue and discuss.
It's really not, though. If I constantly focus on formulas and jerry wants to change the UI, there's little incentive for me to care that jerry is changing the UI when I could just continue to focus on formulas and get stuff done. As long as it still works, it's outside of my wheelhouse and I don't care. That's the general attitude of most contributors of most open-source software. They're issue-driven, not project-driven. General purpose users of open source software should occasionally check in on the software they're using, especially if it's updating on its own.
Currently, they'd need to create an API that provides the expected behavior (meets the contract) and then open a pull request here implementing that API. https://github.com/PathOfBuildingCommunity/PathOfBuilding/blob/dev/src/Classes/ExtBuildListProvider.lua That PR would then need to be accepted, which introduces a bias, and is another flaw of the current implementation. There are suggestions above to change this to a site-agnostic api contract so that users can enter a site address (or multiple) and it would pull from those sites without needing to implement a component in code here. See #7944 (comment) To restate my opinion on the matter: by and large, the feature seems fine and even useful. The problems are with the site. Solution: update the feature so that it's site agnostic and has no bias. Hide the feature until it's site agnostic. |
Even if you make this site agnostic, this is the kind of feature that is ripe for abuse as people figure out how to 'organically' push whatever they want to the top. Combine that with being allowed to create arbitrary titles and it's just ripe for 'grass roots' ads, spam, and other kinds of abuse. The spirit of this idea isn't bad, but the implementation as is right now is terrible, and even a site agnostic version of this is begging for problems and abuse. I think there is a version of this that could exist, but my recommendation would be to pull it out of the software for now and think about what exact problems we think would be worth solving here and how that could be done without opening the door to these kinds of abuse... especially directly monetized ones. |
The returned builds are up to whatever api you're requesting from. If, for example, maxroll were to implement the api, they could return all of or whatever subset of their builds they want. They could only return builds by Zizaran if they wanted, or they could return no builds by Zizaran. It's entirely up to them and their api, and the user is opting in to the experience of maxroll's implementation by adding the maxroll site in their config. Reiterating that this is a hypothetical. Editing to add, as it currently stands, the above behavior would be incredibly problematic by virtue of there being only one site, that is a prescribed default site, that no one opted in to. And also that it's entirely possible with the current implementation. |
From my understanding, part of the problem is there is no API. pobarchives.com is literally hard-coded into PoB. See here: |
An API doesn't have to go over the internet, it just defines how an implementation can interface with the program. Here is the interface that other websites can integrate with to also appear in that list: https://github.com/PathOfBuildingCommunity/PathOfBuilding/blob/18fcd61d17731c0a4b352c752e44090798bc5a55/src/Classes/ExtBuildListProvider.lua It's as hardcoded as the sites used in the Import tab are |
I know what an API is, but thank you. I should have been more clear with my words: another website would have to first submit their own PR with their own code utilizing this API, right? And then have that PR accepted? They couldn't just implement the API server-side and have users paste a url into PoB and get their build lists to show up, right? Because the complaint most people seem to have isn't "there is no way for another website to code against this API" (which would be untrue), but rather "usage of this API requires each build site (and potentially each build creator if they aren't part of a larger network) to write their own lua class, submit that as a PR, and have that PR merged". Which is a much larger barrier to entry than an over-the-wire API that didn't require that degree of coordination with the developers of PoB. |
Yeah, there's definitely room for improvement on ease of entry. Something more akin to this is probably what can be moved towards: https://github.com/PathOfBuildingCommunity/PathOfBuilding/blob/dev/src/Modules/BuildSiteTools.lua#L10 |
I have a stupid question (from someone who as not looked at the code and as never contributed to PoB) would it be possible for someone to add malicious code to a PoB build? Like how sure are you guys that a 3rd party integration can't just execute code on my machine through PoB or this feature? I don't know how the trending builds are loaded into PoB but if the build is loaded and evaluated wouldn't that be dangerous for code injection? |
That would be dangerous, but that isn't what's happening @falingsumo . In simple terms, the data is requested and returned as text. The text is treated as data, and never used in a way that it could be treated as instructions (code) instead. As a result, even if the malicious API returned "delete C:", POB would list a build called "delete C:", not actually carry out that instruction (this is extremely simplified and wouldn't actually work quite this way). It could be possible for people to submit a PR to Path of Building that contains a backdoor / malicious code, and you're at the mercy of the maintainers to correctly review the pull requests to prevent that from happening. This is exactly the same with every open source project in existence. |
0x42 is right. Right now with Lua you can only execute arbitrary code with three methods only. https://www.lua.org/pil/8.html
|
@Wires77 That is not an api but an extension point. An api does not require code changes to the thing providing the api. If you are thinking people need to submit a PR and write new code to add into POB then that is not an API. While an API doesn't have to go over the internet it most commonly is used over the internet. Engineers use APIs of precompiled libraries which is an example of not over the internet. This method though only works when the user of API is writing the host app. In this instance PoB is the host app. So then the only option not internet based for API is to provide a plugin API. This works by first having PoB providing an plugin API interface. Then people could download a plugin from a given website and drop it into a folder in PoB. Then PoB can scan the plugin folder and pull in the plugin. This would require no code changes in PoB proper for new plugins to be made and distributed. Honestly having some kind of Plugin API in PoB would be cool but also a lot of work to implement both for PoB team and anyone writing a plugin. By far the best way to do this is provide a web api spec probably in an OpenAPI format since that is the standard. https://swagger.io/specification/ Then any website that implements that web api spec can be added into PoB by the user. This avoids an biases from PoB as well as any responsibility from PoB over what sources users use. It has been a while since I used lua but I would be willing to write up a spec and implementation and submit a PR if people think this is an appropriate path. |
Latest / trending builds list is currently disabled, see #7995 |
I feel like everything around merging the 'Trending Builds' advertising feature was a mistake.
The biggest issue is of course that this is thinly veiled advertising for a for profit website. The site has active advertisements and directly profits off other people's content that it scrapes. Path of Building should probably not be funneling money directly to a random entity with a default feature.
The content that it scrapes is also another problem. The site being advertised is clearly just scraping youtube channels and posts on reddit to get builds, this is pretty blatantly seen in the presence of a video that has a whopping 0DPS being one of the first visible builds when looking at the page in Path of Building.
In general that the list of builds is a mix of misreported DPS with clickbait, youtube oriented titles and a literal link to something that isn't a build doesn't inspire a great deal of confidence in the feature, and that's ignoring a lot of other issues I have with the presentation in general.
The list of builds also does not have a way to access where the build originates, the preview button takes you to pobarchives.com and you have to actively scroll down to find the video, which has already exposed you to said advertising.
You also cannot ever turn off 'Similar Builds' as a button, it simply displays forever without being able to try and find builds. I feel like this is also a poor feature in general as 'similar builds' is very vague and doesn't really present enough information on why something is similar at all.
The feature in general is putting a lot of power in the hands of a single entity running a for profit, closed source website that allows people to actively pay to advertise their builds which the layperson will 100% assume is directly affiliated with Path of Building.
As an aside, the operators of the site itself are also actively running a reddit bot that actively attempts to redirect people away from an open source, non-profit, ad-free site (pobb.in) to his own for profit, closed source site that fulfils functionally the same role.
I find it very hard to believe anything about the implementation of this feature was done with any goal other directly feeding people to a very specific website, this seems doubly obvious with the request for the site to be 'default' in the original pull request.
I'm genuinely baffled how anyone thought merging it was a good idea and feel like this implies a need for greater controls on what gets merged into the actual app in general.
They're scrubbing said reddit bot posts now, so for posterity here is a screencap of what the unsolicited reply looked like.
)
The text was updated successfully, but these errors were encountered: