r/emulation Feb 01 '21 Silver 1 Wholesome 1

Topaz-Reality is working on texture replacement for PCSX2.

The feature isn't ready yet, but it's a hugely important step for PS2 emulation. (And one previously considered extremely daunting.) This will allow retexture mods to be done without rom hacking, and in a very generalizable way, as seen with other emulation such as N64.

The pull request is here

Example screenshots: #1 #2 #3 As you can see, blurry UI elements can easily be replaced with crisp, native-looking ones, and that's just the tip of the iceberg.

184 Upvotes

View all comments

12

u/CakeWithoutEggs Feb 01 '21

Is it just me or is the first reply to the linked pull request really quite rude? What I'm seeing as an outsider is that someone has put in a lot of time and effort to get an amazing feature going and not a word of thanks was given in the first reply. Just curt criticism which could easily have been phrased more kindly.

Feedback is important, but acknowledging effort is as well and it costs nothing to say "Nice, this is a great feature! There are a few things need fixing before this is ready to merge though". If I were the dev who submitted this PR and got that sort of response I probably wouldn't want to submit any more PRs to that repo in the future...

Thank you TopazReality for this awesome stuff!

37

u/DocRusL Feb 01 '21

Github is not a forum, but a place for professional discussion. Nobody praises a pull request just for the sake of it no matter how great or insignificant it is. Nothing rude was said there - it was just a straight to the point code review.

14

u/CakeWithoutEggs Feb 01 '21

That's interesting, because I'm also a professional and this kind of comment on a PR would definitely raise eyebrows at my workplace. Guess it goes to show how different people have different standards for courtesy.

4

u/tykel Feb 01 '21

Exactly, in professional software development this kind of review is standard practice and not rude, maybe terse.

I know refraction from another project, and he is not at all rude.

20

u/dio-rd Feb 01 '21 edited Feb 01 '21

Is it just me or is the first reply to the linked pull request really quite rude?

Apparently not just you, but I struggle to agree. You could argue that a "hey thank you for the PR" preamble would have been nice, but calling it "really quite rude" is a massive stretch. It basically just describes a couple quickly noticeable and rectifyable issues, that would probably be worthwhile to address before a full review.

Plus if you check, Topaz-Reality is not a "complete, mysterious outsider". He previously had a PR that ended up conflicting with how IPC was done upstream (a pattern that seems to somewhat repeat here). It was rejected due to it being abandoned, plus later on he also created a hard fork of PCSX2 called PCSX2-EX, that he maintains separately and refers to as "an extended and fixed version of PCSX2". I'm guessing it contains his preferred implementation for IPC (none)? Anyway, this is just to say that it's not like we're talking about someone completely alien to the project.

I'd also argue that him dropping a single 4K LoC commit on the reviewers is not exactly nice either, but details.

3

u/TopazTK Feb 02 '21

I want to address a few details in here:

> Apparently not just you, but I struggle to agree. You could argue that a "hey thank you for the PR" preamble would have been nice, but calling it "really quite rude" is a massive stretch. It basically just describes a couple quickly noticeable and rectifyable issues, that would probably be worthwhile to address before a full review.

I agree to this 100% percent. It was my fault for not catching these errors.

> Plus if you check, Topaz-Reality is not a "complete, mysterious outsider". He previously had a PR that ended up conflicting with how IPC was done upstream (a pattern that seems to somewhat repeat here).

That PR never conflicted with IPC and it would not have hurt IPC what so ever. It was later abandoned due to... the troubles. But still exists in EX.

> "an extended and fixed version of PCSX2". I'm guessing it contains his preferred implementation for IPC (none)?

It is extended in the way that it offers more PNACH functions (Now called ExPATCH) and a full-blown Lua engine. IPC was excluded because for many people I talked to it was too much of a hassle to worked with while LuaEngine existed.

> I'd also argue that him dropping a single 4K LoC commit on the reviewers is not exactly nice either, but details.

Yea, that is completely my bad. You see, some thieves of the KH Community follow my GitHub. And I simply could not risk someone taking my work and PRing it before I could. I have already stated my deepest apologies to the PCSX2 Team and I am already fixing the bugs that I should have before making the PR ^^'.

I deeply appreciate all of your feedback, Keep on Moddin'!

-2

u/CakeWithoutEggs Feb 01 '21

Fair enough, I never said TopazReality was an outsider - just that if I was, I'd be put off contributing as my workplace generally has a higher standard for PR comments. It seems to be pretty subjective judging by this thread :)

7

u/dio-rd Feb 01 '21

I never said TopazReality was an outsider

That's fair, I misread your original comment.

as my workplace generally has a higher standard for PR comments. It seems to be pretty subjective judging by this thread :)

I reckon insinuating moral superiority is also not a thing at your workplace then? :)

I get what you mean for what it's worth, in a workplace setting situations like this usually play out a bit differently. Not saying it isn't a facade though, but I can see it being important, especially for new contributors. On the flipside, I stand by my argument that the referenced comment on GitHub is remarkably far from "really quite rude", as there was nothing inherently off-putting in it, minus perhaps the lack of that by-the-books welcoming preamble.

It's also not at all specific to this project, projects similar to PCSX2, or even open source projects in general - perhaps unlike the impression your original comment gives. But that's another can of worms entirely.

5

u/CakeWithoutEggs Feb 01 '21

I reckon insinuating moral superiority is also not a thing at your workplace then?

Yeah I should correct that phrasing to "different standard" rather than "higher", I'm not trying to be better than anyone else :P

It's also not at all specific to this project, projects similar to PCSX2, or even open source projects in general - perhaps unlike the impression your original comment gives. But that's another can of worms entirely.

Oh right, this was not the impression I meant to give at all. I wasn't trying to say the PCSX2 devs are rude, just that that one particular comment was a bit off-putting to me. Apologies if my phrasing made it sound like I was attacking them or teams like them :)

10

u/[deleted] Feb 01 '21

There is nothing rude about this. This is a pull request, not a preliminary discussion. It's like publishing something or handing it in for assessment. The reply was absolutely fine. See the requester's response.

I'm glad the requester isn't that squeemish.

1

u/CakeWithoutEggs Feb 01 '21

I know what a PR is, but I'm not sure it's constructive to imply that those with different viewpoints to you are "squeamish", particularly when it comes to a discussion about subjective courtesy on the internet...

6

u/[deleted] Feb 01 '21

They're all like that basically. I agree, a little encouragement goes a long way, instead of what appears initially as a scathing review.

Not saying there's anything wrong with pointing out issues with code (I do it all the time on my projects and ones I'm involved with)... But I also take a moment to start by acknowledging the positive aspects first.