GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Hardware unit submerge effect using SDL_RenderGeometry by mesilliac · Pull Request #6826 · wesnoth/wesnoth Conversation
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:
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
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
|
There was an error while loading. Please reload this page.
|
We're actually not that far off being able to set 2.0.18 as the minimum SDL2 version. What remains is:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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 It would be nice to be able to smoothly change the ToD colour between shifts, tho. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@mesilliac Just to confirm: is this something you're thinking of ever coming back to complete? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
This is technically complete. It's just really ugly with all the 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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
#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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Does that mean we're dropping this altogether? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
In the future then, I'll leave a comment stating that for situations like this. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
This is so condescending. That person just wrote a 50-line explanation of what he thinks is going wrong. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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.
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.
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.
Dont worry if it was that simple an angry trobulemaker like me would no longer be a dev for some time. :P |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Side note/reminder for later: when the minimum SDL version is increased for this, then the fix from #8137 can be removed as well. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
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 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.
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:
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
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:
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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.
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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.
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.
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.
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.
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.
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 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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
My thought process at the time was essentially this:
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.
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.
I agree. Do you remember which PR that was?
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.
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.
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).
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.
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". |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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.
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
I kinda figured, but that's still a situation where either I or octalot/stevecotton could deal with it more directly. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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.
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.
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I still think that PRs should be closed and that there should just be policy around that. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
It requires SDL version 2.0.18, so must be enabled by defining ENABLE_RENDERGEOMETRY.
139becb to
aa0bd71
Compare
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.
|
Superceded by #8687 due to rebasing issues. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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.
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.
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.
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.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.