GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Hardware unit submerge effect using SDL_RenderGeometry by mesilliac · Pull Request #6826 · wesnoth/wesnoth

Conversation

@mesilliac

@mesilliac mesilliac commented Jul 2, 2022

Copy link
Copy Markdown
Contributor

I'm putting this here without any intention of merging it, because it depends on SDL 2.0.18.

It does however pretty much complete moving the set of on-the-fly unit sprite effects to hardware manipulations.

This PR has three things:

  1. An API function draw::smooth_shaded to draw a texture to a rectangle with specified colour modifier (including alpha) and UV texture coordinate at each corner. It linearly interpolates these values between the corners. It does this using SDL_RenderGeometry, which is only available in SDL version 2.0.18 and above.
  2. An implementation of the unit submerge effect using the above API function.
  3. A flag ENABLE_RENDERGEOMETRY added as an option in CMake and SCons, to optionally enable the use of the above.

Currently it works. It is, however, quite intrusive, and there are a lot of those ENABLE_RENDERGEOMETRY checks. Also the code it touches is quite likely to change, and might even get completely redone in the future. I don't want to have to maintain this while that happens.

This will however provide a good base if someone wants to implement this later. Either if the minimum SDL version progresses enough that it can be put in as is, or if this section of code becomes stable enough that it can be put in with the #ifdef checks, or if the game moves to a graphics framework supporting fragment shaders. The implementation here should port directly to pretty much any GPU framework.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@github-actions github-actions Bot added Building Build-time issues. CMake Issues involving the CMake build system. SCons Issues involving the SCons build system. labels Jul 2, 2022
@mesilliac mesilliac added the Graphics Issues that involve the graphics engine or assets. label Jul 2, 2022
@Pentarctagon

Pentarctagon commented Jul 2, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

We're actually not that far off being able to set 2.0.18 as the minimum SDL2 version. What remains is:

  • The linux CI jobs are still based on Ubuntu 20.04 rather than 22.04, which has 2.0.20. For that I'm waiting for Mint to update to use 22.04 as its base to I can troubleshoot a couple issues locally. Done.
  • The MacCompileStuff maintained by @hrubymar10 libraries use 2.0.12, but I think (?) it should be as simple as changing the version and sha hash to 2.0.18+ and then re-running the build script. Done.
  • The flatpak runtime org.freedesktop.Platform/x86_64/21.08 has only 2.0.16, so we'd need to wait for 22.08 to be released. Done.
mesilliac reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mesilliac mesilliac removed the Building Build-time issues. label Jul 3, 2022
@Wedge009 Wedge009 added the Postponed Issues which cannot be worked on at this time. label Jul 3, 2022
@mesilliac

mesilliac commented Jul 5, 2022

Copy link
Copy Markdown
Contributor Author

Just a note that improving the time-of-day system using SDL would also require SDL_RenderGeometry. It's currently splitting hexes up into pieces to blend ToD colours together. To do this in hardware, it either needs RenderGeometry or shader support.

It's not a huge problem for base ToD effects, as they just load the blended images once and then display them as normal. It's only problematic for animated ToD overrides, like the FADE_TO_BLACK macro uses. For that specifically i'll probably implement a better screen fade effect, as there is an open feature request for that anyway.

It would be nice to be able to smoothly change the ToD colour between shifts, tho.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Pentarctagon

Pentarctagon commented Sep 3, 2022

Copy link
Copy Markdown
Member

@mesilliac Just to confirm: is this something you're thinking of ever coming back to complete?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mesilliac

mesilliac commented Sep 5, 2022

Copy link
Copy Markdown
Contributor Author

This is technically complete. It's just really ugly with all the #ifdef ENABLE_RENDERGEOMETRY switches, so isn't really maintainable without making SDL 2.0.18 a hard requirement.

The core code should be trivially adaptable whenever that becomes the case. And also the draw code here is the same code that would enable acceleration of lightmap rendering. As such i recommend leaving it open for future use targeted at whenever SDL 2.0.18 can be required without too much bother.

I may or may not be around at that time, but this code shouldn't be too difficult for someone else to adapt.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Pentarctagon

Pentarctagon commented Sep 7, 2022

Copy link
Copy Markdown
Member

Alright - for future reference then, everything in the CI is now at 2.0.18 or later, so that's ready if/when this is revisited.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Wedge009

Wedge009 commented Nov 2, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

#4508 makes a case (admittedly a minor one) for making SDL 2.24.0 the minimum requirement. (:

...although I suppose that's more a packaging requirement than a compilation requirement.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Pentarctagon Pentarctagon closed this Feb 12, 2023
@Wedge009

Wedge009 commented Feb 13, 2023

Copy link
Copy Markdown
Member

Does that mean we're dropping this altogether?

Zireael07 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Pentarctagon

Pentarctagon commented Feb 13, 2023

Copy link
Copy Markdown
Member

Not dropping, just that there's no real reason to keep the PR open since anyone can come back and look at this while closed too.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@CelticMinstrel

CelticMinstrel commented Feb 13, 2023

Copy link
Copy Markdown
Member

While that's true, leaving it open sends a signal that we're still interested, while closing it gives an implication that we're not.

Zireael07 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Pentarctagon

Pentarctagon commented Feb 13, 2023

Copy link
Copy Markdown
Member

In the future then, I'll leave a comment stating that for situations like this.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mesilliac

mesilliac commented Dec 23, 2023

Copy link
Copy Markdown
Contributor Author

FTR this issue was ultimately why i never came back to the project, and why i still now have no intention to do more work than is absolutely necessary to support the work i've already done.

I had in my mind one particular todo to finish the hardware acceleration work, and it was this. It was simply waiting for the required SDL version. The code in the PR completely worked and the only problem with it was that it had ugly ifdefs everywhere to fall back in the case where the SDL version wasn't high enough to support it.

There were three things left to complete the harware acceleration work. This, time-of-day blending which requires this exact solution, and UI blur, which requires SDL 3. Which means the only other thing that can be done right now to complete the work i started also relies on this.

I came back earlier this year to find this had just randomly been thrown in the trash. Meanwhile other junk and terminally-unfinished PRs abound.

This was the link i had to come back to the project, and my motivation to come back and do more work. Seeing it simply thrown away for no reason removed all motivation i had.

I don't even have the goodwill left to list the problems i think are causing a terminal lack of development effort in this project. I came back after ten years or so (i was originally a terrain art contributor, not a coder) because when i installed the steam version on my 4K display under X11 it was completely unplayable, and i was sad to see a game i loved become unplayable in a modern era with changing hardware. I did the main thing i felt would help keep the game alive - upgrading the graphics system to support modern capabilities, showing off both the beautiful pixel graphics and high-res portraits (kitty's art is still amazing) while enabling smooth high-res fonts. I spent all the goodwill i previously had for the game to do this, and i consider it done.

There is... a lot more i could do. But i no longer consider graphics to be the main thing constributing to wesnoth's slow decline. Rather... it's contribution policy, the lack of it, the lack of respect for it when it does exist, a lack of respect and support for addon authors and UMC artists and developers, a lack of onboarding or promoting users, addon authors and new contributors to core contributors/developers... well my list goes on.

There's a lot more i could say but frankly i really don't want to bother arguing about it.

I will of course still be around to support my work and answer questions etc. Maybe i'll have some time in the future to come back and see if there's something else i can do to help keep the game alive. But for now i consider my tasks done.

...And even now i'm wording this carefully so i don't get removed from the list of developers in some petty fit of pique.

demario-w reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Pentarctagon

Pentarctagon commented Dec 23, 2023

Copy link
Copy Markdown
Member

The fact that I closed the PR doesn't mean it's thrown away and forever dead. There's no reason you can't reopen a PR.

I closed this because you disappeared and given your last comments on Discord, I honestly thought you already had no plans of coming back. That you started replying and fixing a bunch of the graphical issues which were discovered in the mean time was a complete surprise to me and I'm grateful for the fixes you made. Other far older PRs are left open because the developer who opened them is still around and active and last I asked they still said they planned to get back to it. If celticminstrel disappeared, then yes I'd start closing his old PRs from 2017, 2018, etc as well if nobody was willing or able to review them and make any necessary fix/updates. If you had at any point said that you were taking a break and planned to return eventually, I would've likewise left this PR open.

I'd like to better understand what went wrong and what we can improve on, but I'm also not a mind reader.

demario-w reacted with thumbs down emoji
All reactions
  • 👎 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Pentarctagon

Pentarctagon commented Dec 27, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Alright, well, I'll come back to this at some point early in the 1.19 dev cycle then, assuming there aren't any other reasons this was marked as a draft.

And for what it's worth, I'm sorry that this is the point we've reached.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Pentarctagon Pentarctagon reopened this Dec 27, 2023
@github-actions github-actions Bot added the Building Build-time issues. label Dec 27, 2023
@demario-w

demario-w commented Dec 27, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

I'd like to better understand what went wrong and what we can improve on, but I'm also not a mind reader.

This is so condescending. That person just wrote a 50-line explanation of what he thinks is going wrong.
You might not be a mind-reader, but your brain doesn't seem to be wired to process explicit feedback either.

Zireael07 reacted with thumbs up emoji CelticMinstrel and doofus-01 reacted with thumbs down emoji
All reactions
  • 👍 1 reaction
  • 👎 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Pentarctagon

Pentarctagon commented Dec 27, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Rather... it's contribution policy, the lack of it, the lack of respect for it when it does exist, a lack of respect and support for addon authors and UMC artists and developers, a lack of onboarding or promoting users, addon authors and new contributors to core contributors/developers... well my list goes on.

is all obviously bad. But I genuinely don't know what led to those being listed or, therefore, what specific things could be done to address them. They're not things I've ever intentionally meant to be done badly.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Hejnewar

Hejnewar commented Dec 29, 2023

Copy link
Copy Markdown
Contributor

First, closing PR os someone gone without word is imo something normal, how can we know when or if they return? Just saying a word before solves this problem completely, just hoping that someone returns is not a good practice in open source, that said your work was very welcome and respected by quiet a few people within the community even if you dont know about it.

That person just wrote a 50-line explanation of what he thinks is going wrong.

Without explanation these are just statemsnts that dont add a lot. What is that lack of respect for example? Just saying that doesnt say what we do wrong, to know that we would actually need to be mindreaders.

There is... a lot more i could do. But i no longer consider graphics to be the main thing constributing to wesnoth's slow decline. Rather... it's contribution policy, the lack of it, the lack of respect for it when it does exist, a lack of respect and support for addon authors and UMC artists and developers, a lack of onboarding or promoting users, addon authors and new contributors to core contributors/developers... well my list goes on.

I feel called out here because I actually work with community a bit and come from very anti dev environoment.

What respect and support are we talking about? Lack of compatybility breaking changes? Answering questions? Actually helping? Unfortunatley we often have very limited time and while we can avoid some often we have to just break something to make the game better.

Its hard to onboard users that dont want to work with mainline or devs, issue that Im somewhat trying to solve druing 1.19 we will see how that turns out (kinda waiting for 1.19 already).

Addon authors and new contributors to core contributors/developers, again same issue as before people dont really want to be devs. It is not easy to solve that but time maybe.

...And even now i'm wording this carefully so i don't get removed from the list of developers in some petty fit of pique.

Dont worry if it was that simple an angry trobulemaker like me would no longer be a dev for some time. :P

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Pentarctagon

Pentarctagon commented Jan 1, 2024

Copy link
Copy Markdown
Member

Side note/reminder for later: when the minimum SDL version is increased for this, then the fix from #8137 can be removed as well.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mesilliac

mesilliac commented Jan 23, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

@Hejnewar

First, closing PR os someone gone without word is imo something normal, how can we know when or if they return? Just saying a word before solves this problem completely

If you look above, there was a comment and reply from me. As i said there, i was unsure whether or not i would be back to work on it, but it was essentially already complete. If the PR is trashed, absolutely nobody is going to find it in the future. Nobody but me will even remember it exists.

I even kept an ear out to see whether it would be mentioned when i came back and did some related things recently. It was not.

I feel called out here because I actually work with community a bit and come from very anti dev environoment.

I have absolutely no problem with you. In fact i noticed several times you proposing clear and important improvements and those proposals not being worked on. In my opinion you are on the other side here.


@Pentarctagon

The fact that I closed the PR doesn't mean it's thrown away and forever dead. There's no reason you can't reopen a PR.

Yeah, i can dumpster dive into the trash and pull my work out. I'm not fucking going to. As i said it was complete and easy for somebody else to integrate when the SDL requirement is bumped. Nobody is ever going to find a closed PR they have no idea exists.

It seemed to me (and still does) you were offended by the way i left and wanted to sever any connection. You absolutely would have closed this if i wasn't there to respond immediately the first time. I responded... so you waited a few months and stealthily closed it without asking this time so there was no chance i could reply and say it was worth keeping. Not that i should have needed to... because i already did.

In fact this is literally what i said:

The core code should be trivially adaptable whenever that becomes the case. And also the draw code here is the same code that would enable acceleration of lightmap rendering. As such i recommend leaving it open for future use targeted at whenever SDL 2.0.18 can be required without too much bother.

I may or may not be around at that time, but this code shouldn't be too difficult for someone else to adapt.


As i said before i really don't want to argue. I don't want to point fingers. I don't think there's anyone who doesn't want the project to do well. I just lack the goodwill to write the essay needed to elaborate on everything else.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Pentarctagon

Pentarctagon commented Jan 24, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

I've... never been offended by anything you've done. The improvements you've made to Wesnoth are amazing, and as far as I could tell people were broadly supportive of and enthusiastic about the changes you were making. I closed this PR because:

  • You went on a mini-rant on Discord, then completely disappeared not that long after.
  • The PR was (and still is) marked as a Draft, which as per Github's meaning of that is supposed to be used to indicate that a PR isn't ready to be merged in its current state and needs further changes. If a PR is complete but waiting on some other external limitation to be resolved, then that's what the Postponed tag is for (and this PR is both Postponed and a Draft for some reason).
  • I took the statements "This will however provide a good base if someone wants to implement this later" and "This is technically complete ... The core code should be trivially adaptable whenever that becomes the case" to mean that was a base implementation that could be merged as-is, but also wasn't entirely complete.
  • Closing PRs where the author has been unresponsive for a while isn't something new for me to do - I've done the same for dozens of other PRs that were never finished and the author disappeared. This isn't something I singled you out for, it's something I do globally to try and keep the PR list from endlessly growing with stuff people have submitted and seem unlikely to ever come back to complete.
  • Closed PRs can be re-opened. They aren't tossed into a void and gone forever, and me closing a PR isn't some sort of pronouncement that it's forever dead or that it's not wanted. They can be re-opened with a click of a button for pretty much any reason aside from an explicit statement saying the PR isn't wanted - anyone who comes back after a while and sees their PRs were closed in the mean time can just... re-open them.

I also think that perhaps you don't really understand how much more knowledgeable you are on this area compared to pretty much anyone else involved with the project. For #6960 I'm just flailing around in the dark, currently trying to figure out why something that seems to solve the problem even works, because nobody else seems to have much of an idea either. For this PR all I can really do with it if you're not around is strip out the ENABLE_RENDERGEOMETRY bits, resolve the merge conflicts hopefully correctly, and then see if it causes any issues - if it does and the fix isn't something trivial/obvious, then it'd end up reverted anyway for my lack of understanding of how basically any of Wesnoth's rendering works and a lack of time to try to learn it all by myself.

Also, I suppose, just to call it out - you do still have commit access here on Github, you're still in the Developers group on the forums, and you're still in the Developers & Artists group on Discord. Were I intending to sever any connection with you, that wouldn't be the case for any of those places.

Zireael07 and Hejnewar reacted with thumbs up emoji demario-w reacted with thumbs down emoji
All reactions
  • 👍 2 reactions
  • 👎 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Hejnewar

Hejnewar commented Jan 28, 2024

Copy link
Copy Markdown
Contributor

Yeah, i can dumpster dive into the trash and pull my work out. I'm not fucking going to. As i said it was complete and easy for somebody else to integrate when the SDL requirement is bumped.
If the PR is trashed, absolutely nobody is going to find it in the future. Nobody but me will even remember it exists.

It is really easy to filter it out with just postponed label among closed PRs, and yes people do that sometimes here from what I can see. Somewhat of a meta knowledge I guess.

It seemed to me (and still does) you were offended by the way i left and wanted to sever any connection. You absolutely would have closed this if i wasn't there to respond immediately the first time. I responded... so you waited a few months and stealthily closed it without asking this time so there was no chance i could reply and say it was worth keeping. Not that i should have needed to... because i already did.

Thats basically consipracy theory, and not even a good one because I could just disprove it looking at what people do and by the fact that removing the postponed label was never removed which would make it significantly harder to find.

[instert many PRs bad argumenst] so keeping many PRs open bad. Unfortunatley no thing such as frezze exists in git. Postponed + closed is as good as we are getting. So clam down because what you are saying is not true and I can guerantee that.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mesilliac

mesilliac commented Jan 28, 2024

Copy link
Copy Markdown
Contributor Author

Thats basically consipracy theory, and not even a good one because I could just disprove it looking at what people do and by the fact that removing the postponed label was never removed which would make it significantly harder to find.

It should not even be possible for such a theory to be valid. Look at the sequence of events and tell me how it would be different if this were actually the case. The postponed label being there or not is irrelevant.

You are both still completely ignoring the explicit recommendation i made to keep it open. If there is some reason to override that recommendation, it should be stated. At worst how it was handled indicates duplicitous behaviour. At best it indicates a lack of respect for someone's contributions, not even bothering to address why it was left open in the first place, or the direct communication that already existed.

I took the statements "This will however provide a good base if someone wants to implement this later" and "This is technically complete ... The core code should be trivially adaptable whenever that becomes the case" to mean that was a base implementation that could be merged as-is, but also wasn't entirely complete.

You are capable of attempting to clarify. How do you excuse completely ignoring the statement following that?

Even now this discussion is not an attempt to figure out what was wrong - it's a concerted attempt to deny any wrongdoing and absolve all responsibility.

The best possible interpretation here is that you somehow didn't read the message history before closing the issue, and somehow didn't remember the previous communication. And even that shows a lack of respect for contributions in not even bothering to check why something was left open.

This whole argument is just excuses, denials, and avoiding the question of why on earth this could ever be a problem in the first place.

Closed PRs can be re-opened. They aren't tossed into a void and gone forever, and me closing a PR isn't some sort of pronouncement that it's forever dead or that it's not wanted. They can be re-opened with a click of a button for pretty much any reason aside from an explicit statement saying the PR isn't wanted - anyone who comes back after a while and sees their PRs were closed in the mean time can just... re-open them.

As noted by others in this very thread, closing the PR indicates that the contribution is not wanted. Closing it without comment indicates that you didn't even respect the contributor enough to check it with them, and here not even enough to read the previous messages in this thread.

Closing PRs where the author has been unresponsive for a while isn't something new for me to do - I've done the same for dozens of other PRs that were never finished and the author disappeared. This isn't something I singled you out for, it's something I do globally to try and keep the PR list from endlessly growing with stuff people have submitted and seem unlikely to ever come back to complete.

And how many pontetial new devs had their contributions ignored and then silently closed later and just took it as a "we don't want you goodbye"? When i first started working on the graphics stuff i noticed a potential dev make a contribution which was ignored for weeks and later quietly closed.

This kind of thing is exactly what i mean when i say there's a lack of respect for new developers and a lack of attempts to attract them. I've seen public statements by you that the Wesnoth project is critically understaffed. This is exactly why it should be doing whatever it can to encourage potential new developers, not discourage them.

I also think that perhaps you don't really understand how much more knowledgeable you are on this area compared to pretty much anyone else involved with the project. For #6960 I'm just flailing around in the dark, currently trying to figure out why something that seems to solve the problem even works, because nobody else seems to have much of an idea either.

If my skills are so irreplacable, that is even more reason to try to encourage not discourage my contribution. I did not leave this PR here expecting one of the current team to complete it (even though i know there is a competent graphics programmer on the team). This type of issue is perfect to attract new dev talent. A uni graduate who specialized or even dabbled in CG could pick this up and implement it. And yes, before some recent mildly unnecessary changes were made to how draw calls work, it would have just been a matter of removing all the ifdefs. The project would then have a potential skilled dev who has gained some familiarity with the codebase via a problem which has already been basically solved.

You keep saying "it can just be reopened", but that is not the point. The point is that if it is closed nobody will find it, and if there is a reason to close it that should be clearly stated here. No such reason was given. It was just closed, and the message this sends is "goodbye, we don't even care enough to explain".

If it is policy that contributions are closed after a certain amount of time not hearing back from a dev, then point that out. There of course was no such time limit. For the earlier query i did instantly respond, and for the later... there was no query.

In stead what the appearance is, in this project, it's that there is a core clique of priveleged developers. (And i am fully aware that on the balance of things i am in this core clique.) This core clique is allowed to keep their junk as PRs in the main respository, commit whatever they like to the master branch without review, and generally treat the wesnoth master branch as their own personal toybox. Meanwhile potential new developers have their PRs closed without explanation and may or may not get any response to begin with. Addon authors who have their work trashed by random changes by a clique member are just left out to dry.

I don't think this is a problem with individuals. It's a problem with policy.

One of the biggest things that turned me off the project was having to spend several hours in Discord arguing just so that unnecessary changes weren't made in complete and direct violation of existing, codified and ratified deprecation policy. This should never be a thing. I should never have to argue this. There should be no argument. There was a clear policy in place just so people's work wasn't randomly trashed without warning, and i was literally having to argue that such policy should not be willfully ignored.

The desire of a core dev to make those changes in the first place, even after being told the repurcussions and the policy, indicates an ingrained lack of respect or care for those people who would have their work damaged by the change. And again i am aware that this doesn't describe everyone and there are definitely people in the project doing their best to prevent such things from happening. But they shouldn't have to be fighting for it. It should simply be policy to deal with these things the correct way, and where it is policy, that policy should be followed. Even without policy, it should be just a matter of basic decency to respect the contributions and hard work of others.

[instert many PRs bad argumenst] so keeping many PRs open bad.

Nobody has presented any such argument here. It was simply closed, and two messages from others noting that this indicates the contribution is unwanted were summarily brushed off.

If this PR were my own personal WIP, then it should absolutely 100% not be hanging around in the list of wesnoth PRs. But it was not. It was a contribution to the project, hence me submitting it as a PR. My own personal WIPs are in my own personal WIP branches. It was marked as draft because i did not think it should be merged with the ugly ifdef mess and additional ENABLE_RENDERGEOMETRY define that would become unnecessary with a SDL version requirement bump. I explained this as clearly as i could in the OP. I did not mark it "postponed" and have no idea what official meaning that may or may not have. In fact i had to look into github docs just to figure out how to find my own PR.

What i am not sure how you can justify ignoring, is that it felt to me based on this as if my contribution, both specifically and in general was unwanted. My explicit wishes were simply ignored.

I am not demanding an apology or saying you should beg me to contribute. I am saying that this kind of interaction drives developers and contributors away, and i believe this is the biggest issue the project currently faces. Wesnoth should be attracting and nurturing developers and contributors at all levels, not merely permitting them.

If i cared just a little bit less i wouldn't even have said anything. Even now i just have unpleasant interaction to show for it.

demario-w reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Pentarctagon

Pentarctagon commented Jan 28, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

You are capable of attempting to clarify. How do you excuse completely ignoring the statement following that?

Even now this discussion is not an attempt to figure out what was wrong - it's a concerted attempt to deny any wrongdoing and absolve all responsibility.

The best possible interpretation here is that you somehow didn't read the message history before closing the issue, and somehow didn't remember the previous communication. And even that shows a lack of respect for contributions in not even bothering to check why something was left open.

This whole argument is just excuses, denials, and avoiding the question of why on earth this could ever be a problem in the first place.

My thought process at the time was essentially this:

  • The author of this PR has been unresponsive for a while now, so trying to contact them here, via Discord, or via the forums is likely to continue to be unsuccessful.
  • In the past, trying to reach out to otherwise unresponsive project members via email has been unwelcome, so I shouldn't try that.
  • In my subjective interpretation of the messages, I wasn't completely confident that this was ready to merge as-is.
  • I didn't trust myself to understand the change well enough to judge whether it was ready to merge, nor to fix any problems if issues arose.
  • A different person asking if they can start working on someone else's old PR has simply never happened, whether for old PRs that are still open because the developer who opened it is still active or back before I started closing old PRs at all.

Therefore, as I understood the situation, there was essentially no chance of this PR being able to be merged, so I closed it. If I'm an idiot for thinking those things then fine, but that was my thought process.

As noted by others in this very thread, closing the PR indicates that the contribution is not wanted. Closing it without comment indicates that you didn't even respect the contributor enough to check it with them, and here not even enough to read the previous messages in this thread.

Yes, and I was called out for that by others. To which I said I would make sure to leave an explanation comment in the future, which I have done to explicitly state that those PRs can be re-opened if the author returns to it.

And how many pontetial new devs had their contributions ignored and then silently closed later and just took it as a "we don't want you goodbye"? When i first started working on the graphics stuff i noticed a potential dev make a contribution which was ignored for weeks and later quietly closed.

This kind of thing is exactly what i mean when i say there's a lack of respect for new developers and a lack of attempts to attract them. I've seen public statements by you that the Wesnoth project is critically understaffed. This is exactly why it should be doing whatever it can to encourage potential new developers, not discourage them.

I agree. Do you remember which PR that was?

If my skills are so irreplacable, that is even more reason to try to encourage not discourage my contribution. I did not leave this PR here expecting one of the current team to complete it (even though i know there is a competent graphics programmer on the team). This type of issue is perfect to attract new dev talent. A uni graduate who specialized or even dabbled in CG could pick this up and implement it. And yes, before some recent mildly unnecessary changes were made to how draw calls work, it would have just been a matter of removing all the ifdefs. The project would then have a potential skilled dev who has gained some familiarity with the codebase via a problem which has already been basically solved.

You keep saying "it can just be reopened", but that is not the point. The point is that if it is closed nobody will find it, and if there is a reason to close it that should be clearly stated here. No such reason was given. It was just closed, and the message this sends is "goodbye, we don't even care enough to explain".

At least for Wesnoth, a different person picking up another person's old PR and finishing it themselves simply isn't a thing that happens. Previously in the case where an active developer is no longer active and leaves open PRs behind, I have gone through and tried to salvage the ones that are simple enough for me to figure out or were in areas I was already relatively familiar with, but that's the only time that happens.

What does actually happen is a new contributor will look at the Good First Issue tag and pick something from that list to start with.

If it is policy that contributions are closed after a certain amount of time not hearing back from a dev, then point that out. There of course was no such time limit. For the earlier query i did instantly respond, and for the later... there was no query.

In stead what the appearance is, in this project, it's that there is a core clique of priveleged developers. (And i am fully aware that on the balance of things i am in this core clique.) This core clique is allowed to keep their junk as PRs in the main respository, commit whatever they like to the master branch without review, and generally treat the wesnoth master branch as their own personal toybox. Meanwhile potential new developers have their PRs closed without explanation and may or may not get any response to begin with. Addon authors who have their work trashed by random changes by a clique member are just left out to dry.

I don't think this is a problem with individuals. It's a problem with policy.

What essentially happened was a senior developer made a big deal about how not closing old/unfinished/abandoned PRs makes Wesnoth look like it's abandoned even though those PRs will never be in a state to merge anyway, so I said fine I can see how that makes sense and that I'd go through them periodically and prune them. You are correct though that this should've been made a formal policy.

One of the biggest things that turned me off the project was having to spend several hours in Discord arguing just so that unnecessary changes weren't made in complete and direct violation of existing, codified and ratified deprecation policy. This should never be a thing. I should never have to argue this. There should be no argument. There was a clear policy in place just so people's work wasn't randomly trashed without warning, and i was literally having to argue that such policy should not be willfully ignored.

The desire of a core dev to make those changes in the first place, even after being told the repurcussions and the policy, indicates an ingrained lack of respect or care for those people who would have their work damaged by the change. And again i am aware that this doesn't describe everyone and there are definitely people in the project doing their best to prevent such things from happening. But they shouldn't have to be fighting for it. It should simply be policy to deal with these things the correct way, and where it is policy, that policy should be followed. Even without policy, it should be just a matter of basic decency to respect the contributions and hard work of others.

Even core developers aren't immune to wanting to put something in that they think is worth it despite the breakage (some more than others). Situations like that are also part of what we have the project council for. For what it's worth, 1.18 has relatively few known breakages with 1.16 (https://forums.wesnoth.org/viewtopic.php?t=57288).

Nobody has presented any such argument here. It was simply closed, and two messages from others noting that this indicates the contribution is unwanted were summarily brushed off.

I'm not sure why you think I brushed off those comments - in response to those comments I said that this wasn't closed because we were rejecting the proposed change and acknowledged I should state that going forward.

If this PR were my own personal WIP, then it should absolutely 100% not be hanging around in the list of wesnoth PRs. But it was not. It was a contribution to the project, hence me submitting it as a PR. My own personal WIPs are in my own personal WIP branches. It was marked as draft because i did not think it should be merged with the ugly ifdef mess and additional ENABLE_RENDERGEOMETRY define that would become unnecessary with a SDL version requirement bump. I explained this as clearly as i could in the OP. I did not mark it "postponed" and have no idea what official meaning that may or may not have. In fact i had to look into github docs just to figure out how to find my own PR.

What i am not sure how you can justify ignoring, is that it felt to me based on this as if my contribution, both specifically and in general was unwanted. My explicit wishes were simply ignored.

I am not demanding an apology or saying you should beg me to contribute. I am saying that this kind of interaction drives developers and contributors away, and i believe this is the biggest issue the project currently faces. Wesnoth should be attracting and nurturing developers and contributors at all levels, not merely permitting them.

If i cared just a little bit less i wouldn't even have said anything. Even now i just have unpleasant interaction to show for it.

They were ignored because I nevertheless managed to misunderstand them anyway, but also because taking a look through what's left behind after a developer becomes unresponsive/inactive and trying to determine what can be fixed up and merged vs what probably can't is just something I do as project lead. Were it not marked as a draft, I probably would've left it open to at least attempt to merge it into 1.19 myself. Had you said you planned to come back to it in a few months or a year, then likewise I would've left it open, since I would've known you still planned to work on it.

I didn't go into this thinking "fuck that guy he offended me", I went into it thinking "well, he hasn't responded to any attempt to ping/message him in months, I guess I need to look through what's left behind and see if there's anything I feel confident enough to merge myself and thereby take responsibility for any issues it might cause".

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Hejnewar

Hejnewar commented Jan 28, 2024

Copy link
Copy Markdown
Contributor

Even core developers aren't immune to wanting to put something in that they think is worth it despite the breakage (some more than others). Situations like that are also part of what we have the project council for. For what it's worth, 1.18 has relatively few known breakages with 1.16 (https://forums.wesnoth.org/viewtopic.php?t=57288).

It was Vultraz, so yeah invoking PC in that case might not be the best. I remember that because I was like "oh Im gonna [...] if that happens again" after reading that, unfortunatley IIRC I was late to the party that time and you might not know but I have some history of doing that already.

I am not demanding an apology or saying you should beg me to contribute. I am saying that this kind of interaction drives developers and contributors away, and i believe this is the biggest issue the project currently faces. Wesnoth should be attracting and nurturing developers and contributors at all levels, not merely permitting them.
If i cared just a little bit less i wouldn't even have said anything. Even now i just have unpleasant interaction to show for it.

Honestly Im sick of the accusations and pointing fingers already, so stop, Penta even admitted fault already. If what you say in this text above true then stop being hanged up on the past and focus on the future and making it better. It is possible and it can be done, you can help fix the things instead. Next dev cycle is comming up soon, sounds like perfect time.

demario-w reacted with thumbs down emoji
All reactions
  • 👎 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Pentarctagon

Pentarctagon commented Jan 28, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

It was Vultraz, so yeah invoking PC in that case might not be the best. I remember that because I was like "oh Im gonna [...] if that happens again" after reading that, unfortunatley IIRC I was late to the party that time and you might not know but I have some history of doing that already.

I kinda figured, but that's still a situation where either I or octalot/stevecotton could deal with it more directly.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mesilliac

mesilliac commented Jan 28, 2024

Copy link
Copy Markdown
Contributor Author

I have repeatedly said i don't want to point fingers or make this into an argument. This is not about whether or not the PR should have been closed. Whether it should be closed or not is irrelevant.

This is about the way it was closed, directly ignoring the explicit statement "As such i recommend leaving it open for future use targeted at whenever SDL 2.0.18 can be required without too much bother." and offering no explanation or justification at all.

I clearly said "I may or may not be around at that time, but this code shouldn't be too difficult for someone else to adapt.". This means... i may or may not be around, but i believed it would be useful to keep around, hence leaving it open.

A simple statement of "I don't think anyone else will be abe to pick this up unless you do it, do you think you will be able to finish this in the future?" would have completely bypassed this entire mess. As would some sort of response when i said it, like "I don't want to keep draft projects around as they tend to just clutter up the repository and nobody finishes them". None of this was communicated at all until the damage was already well done.

I don't know how different ways i can say this. Again, i'm not doing this to point fingers or assign blame. I'm pointing out that this is the final reason why i have no motivation to continue. It may be just a last straw, but i think it illustrates a cultural disregard for people's hard work and contributions which is detrimental to the project. Whether or not there was any actual negative intent is irrelevant. My point is that general policy should prevent anything like this from occurring in the first place.

There are plenty of ways that this could have been handled that would not have left me with negative feelings regarding this particular contribution. And in the end those negative feelings led to a lack of desire to finish it myself, and thus a lack of desire to make any other contributions when the other major unfinished hardware rendering task also relied on it.

All developers and contributors have some reason they contribute. They have some internal currency that causes them to donate effort, and gives them motivation to continue. Whether it's pride in their contribution, pride in their work, praise, fame/reputation, ego, whatever. If i was being paid to do this, i would just pick this back up and implement it. But i'm not - that currency has to come from somewhere else, and in this case, it could have been there if this were handled differently, but wasn't.

This is also why i don't want to argue about it. Because i don't want to discourage the contributions of others either. I think everyone on this project is doing good work and i don't want anyone to think i'm accusing them of driving me away. I'm not. I'm simply pointing out that this is something i believe the project needs to actively change and improve on.

I've mentioned before that i don't have all that much more time to contribute, so it's not even a particularly big loss. But there could have been a cultivation of the connection. And yes there were people attempting to cultivate that connection, but... it just didn't overcome the negative sentiment from this and other things prior.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mesilliac

mesilliac commented Jan 29, 2024

Copy link
Copy Markdown
Contributor Author

I agree. Do you remember which PR that was?

I couldn't find it when i searched, and don't really remember any details. I just remember noticing it and thinking it was off at the time.

I'm not sure why you think I brushed off those comments - in response to those comments I said that this wasn't closed because we were rejecting the proposed change and acknowledged I should state that going forward.

There are a lot of ways such a comment could be left that still wouldn't change the sentiment at all. Even with that acknowledgement, there was no attempt to provide any justification for why this PR was closed. Brushing concerns off goes just like that.

A different person asking if they can start working on someone else's old PR has simply never happened, whether for old PRs that are still open because the developer who opened it is still active or back before I started closing old PRs at all.

When i came in to do the graphics system upgrade, the very first thing i did was go look for old work that had previously been done and see if i could adapt it. In the end it wasn't really feasible, but i definitely tried.

If somebody came and wanted to implement say smooth-fade for time-of-day lighting, they would need this solution, or else to reimplement it. If it were hanging around as a PR, chances are somebody could point them at it. If it's closed and buried, chances of that are very very low.

At this point i definitely think it should be closed as it's just a lot of negativity that a new developer really shouldn't be exposed to, and i have no intention of updating it for the modified drawing calls.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@CelticMinstrel

CelticMinstrel commented Jan 29, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

At this point i definitely think it should be closed as it's just a lot of negativity that a new developer really shouldn't be exposed to, and i have no intention of updating it for the modified drawing calls.

We could probably mark all these comments as hidden to somewhat avoid exposing potential new developers to this rather downputting conversation. They'd still be able to see it if they really wanted to, I think, but it wouldn't be right in their face.

The only other option I'm aware of would be to close this and reopen it as a new pull request. I'm not sure if that requires the source branch to be different though. But it could be worth a try.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Pentarctagon

Pentarctagon commented Jan 29, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

I believe I've explained quite thoroughly why I did what I did, so I don't think there's much else I can say about that either at this point. The one concrete change I can make is to just stop closing PRs, since I'm now aware that people may be majorly put off by that when coming back after a long period of absence. The limited number of breaking changes between 1.16 and 1.18 does, I think, already show good progress made on that issue as well.

Beyond that, I'm not really sure, though I disagree that we generally show disregard for other people's contributions. I do know it can seem that way given that PR reviews can sometimes take a while to be provided or even on occasion be missed entirely, but I'm not sure what can be done about that - people do reviews as they have time to do them, and while I do periodically look through PRs for ones that are waiting on us to do something I do still miss things. Otherwise, a lack of specific examples makes it difficult to know what other problems you're seeing.

doofus-01, Hejnewar, and soliton- reacted with thumbs up emoji
All reactions
  • 👍 3 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Hejnewar

Hejnewar commented Jan 29, 2024

Copy link
Copy Markdown
Contributor

I still think that PRs should be closed and that there should just be policy around that.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@cooljeanius cooljeanius added this to the 1.19 milestone Mar 10, 2024
@Wedge009 Wedge009 mentioned this pull request Mar 26, 2024
SDL3 #8638
Merged
@Pentarctagon Pentarctagon removed the Postponed Issues which cannot be worked on at this time. label Apr 1, 2024
@mesilliac @Pentarctagon
A drawing routine for smooth-shaded texture drawing
aa0bd71
It requires SDL version 2.0.18, so must be enabled by defining
ENABLE_RENDERGEOMETRY.
@Pentarctagon
Pentarctagon force-pushed the draw_smooth_shaded branch from 139becb to aa0bd71 Compare April 3, 2024 04:15
Pentarctagon added a commit to Pentarctagon/wesnoth that referenced this pull request Apr 3, 2024
@Pentarctagon
Hardware unit submerge effect using SDL_RenderGeometry.
0df9871
Original code from mesilliac in wesnoth#6826. Made a separate PR for this since the relevant code moved to an entirely separate file and trying to "properly" fix the merge conflicts resulting from that isn't worth the time.
@Pentarctagon Pentarctagon mentioned this pull request Apr 3, 2024
Hardware unit submerge effect using SDL_RenderGeometry. #8687
Merged
@Pentarctagon

Pentarctagon commented Apr 3, 2024

Copy link
Copy Markdown
Member

Superceded by #8687 due to rebasing issues.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Pentarctagon Pentarctagon closed this Apr 3, 2024
Pentarctagon added a commit to Pentarctagon/wesnoth that referenced this pull request Apr 4, 2024
@Pentarctagon
Hardware unit submerge effect using SDL_RenderGeometry.
858f5c3
Original code from mesilliac in wesnoth#6826. Made a separate PR for this since the relevant code moved to an entirely separate file and trying to "properly" fix the merge conflicts resulting from that isn't worth the time.
Pentarctagon added a commit to Pentarctagon/wesnoth that referenced this pull request Apr 4, 2024
@Pentarctagon
Hardware unit submerge effect using SDL_RenderGeometry.
570629b
Original code from mesilliac in wesnoth#6826. Made a separate PR for this since the relevant code moved to an entirely separate file and trying to "properly" fix the merge conflicts resulting from that isn't worth the time.
Pentarctagon added a commit to Pentarctagon/wesnoth that referenced this pull request Apr 10, 2024
@Pentarctagon
Hardware unit submerge effect using SDL_RenderGeometry.
5dbeecb
Original code from mesilliac in wesnoth#6826. Made a separate PR for this since the relevant code moved to an entirely separate file and trying to "properly" fix the merge conflicts resulting from that isn't worth the time.
Pentarctagon added a commit that referenced this pull request Apr 11, 2024
@Pentarctagon
Hardware unit submerge effect using SDL_RenderGeometry. (#8687)
9548f4c
Original code from mesilliac in #6826. Made a separate PR for this since the relevant code moved to an entirely separate file and trying to "properly" fix the merge conflicts resulting from that isn't worth the time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

Building Build-time issues. CMake Issues involving the CMake build system. Graphics Issues that involve the graphics engine or assets. SCons Issues involving the SCons build system.

Projects

None yet

Milestone

1.19

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@mesilliac @Pentarctagon @Wedge009 @CelticMinstrel @demario-w @Hejnewar @cooljeanius
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.