raduprv / Eternal-Lands

http://www.eternal-lands.com
Other
155 stars 56 forks source link

Can we do a 1.9.6.1 (p1) release build? #175

Closed pjbroad closed 2 years ago

pjbroad commented 2 years ago

There are a number of big branches to merge and there's the GLES3 proposal. Before we do those, can we do a minor release updated to build 1? There have been a number of fixes applied since the 1.9.6.0 release and we have some minor updates to data too. I'm not taking about a long drawn out process, it can be done really quickly I think.

raduprv commented 2 years ago

The GLES3 branch would be a long time thing, it's a matter of months probably. I guess it should be a separate branch and merged when it's done. But sure, having another minor release is not a problem.

gvissers commented 2 years ago

Let's do a release, there's quite a few paper cuts fixed.

Do we want to include #169 and/or #171? Not sure if these count as a bugfix, and they haven't had much testing. The changes aren't all that scary though, and I personally find them useful. Might want to skip #169 if we do #171 though. (Ideally we'd highlight the result of the pathfinder if it finds one, but that change would be more intrusive).

What about #167? Not sure if it fixes anything for other people currently having issues with water reflection, but it Works For Me(tm) and should have no effect if your OpenGL is not topsy turvy.

raduprv commented 2 years ago

I think we should include the water shader fix. On a slightly unrelated note, I tried the client in VirtualBox (latest version) on a Windows 10 host, Linux Mint guest, with 3d acceleration enabled, and it didn't work (it only rendered the sky properly, the rest was black). For the tile/pathfinding thing, maybe we should release a client with it but have it configurable, to be able to turn those things off.

gamil-zirak commented 2 years ago

If we do #171 we should skip #169 because that would change how I'd want to do #169. There is still an open question about considering minimum distance in #171. We should decide what to do there because it can introduce an inconvenience to harvesters in certain circumstances.

pjbroad commented 2 years ago

That all sounds good. I'll do more the testing on those pull requests too. @bendoughty are you up for a MacOS build? It would be good to test the water shader fix on Mac too. I think the only other changes since release that might be different for Mac are the main window resizing fix. I used the relevant SDL calls that should be fine on a high DPI display. And the fix for enabling power saving.

bendoughty commented 2 years ago

@bendoughty are you up for a MacOS build?

You bet, I'll get to work on a small PR for the Xcode stuff right now.

It would be good to test the water shader fix on Mac too. I think the only other changes since release that might be different for Mac are the main window resizing fix. I used the relevant SDL calls that should be fine on a high DPI display. And the fix for enabling power saving.

I have only taken the client for a quick spin, but everything seems to be working as expected so far!

bendoughty commented 2 years ago

The PR is ready (#176) in the meantime I'll keep testing the changes.

gvissers commented 2 years ago

The PR is ready (#176)

Merged, thanks.

in the meantime I'll keep testing the changes.

I have just merged the water shader fixed mentioned earlier. Note that the shader file itself (shaders/reflectiv_water_fs.glsl) was also changed, you will have to update the shader in your EL data directory to test the effect (there should be none if water reflections are already working for you).

bendoughty commented 2 years ago

I have just merged the water shader fixed mentioned earlier. Note that the shader file itself (shaders/reflectiv_water_fs.glsl) was also changed, you will have to update the shader in your EL data directory to test the effect (there should be none if water reflections are already working for you).

Thanks for the guidance. I have given this a try, and can report no change in the (already correct) behaviour on any of the AS or Intel-based Macs I have to hand.

feeltheburn commented 2 years ago

@pjbroad @bendoughty (anyone else does builds?)

!!! If it's not too late, please grab this and replace it in /maps/ for this build.

https://el-db.com/tempstuff/guildmap_lion.elm.gz

(Last minute work done and shop paid. No biggie if it's too late.. at least grab it for any future builds then.)

bendoughty commented 2 years ago

Sorry, @feeltheburn, I am afraid that I am the wrong person to ask about data changes. My macOS builds use the data pack attached to the corresponding GitHub release in order to ensure parity with other platforms.

FWIW I know that there is at least one more minor code change on the way (and the version number has not been bumped yet) so I doubt this change has come too late to make the release.

BTW: I have been thoroughly enjoying the new tab maps you created for 1.9.6, you're a talented guy!

pjbroad commented 2 years ago

!!! If it's not too late, please grab this and replace it in /maps/ for this build. https://el-db.com/tempstuff/guildmap_lion.elm.gz

We've not done a data pack yet so no problem including this. We also have the shader file and item_extra_info.txt changes.

feeltheburn commented 2 years ago

BTW: I have been thoroughly enjoying the new tab maps you created for 1.9.6, you're a talented guy!

Perl scripts FTW ;) A 3300+ line perl script made by me, with imported code (esp. "elm2pov", an abandoned script written by someone on the French server that makes the base tab map from the elm file) and my extensive MySQL database and ImageMagick make the entire map creation system automated, for both of my tab map types. It also spews out a version just for me that helps me debug maps, including every secret and guild map entrance so I don't accidentally remove them while editing a certain map area. (Learner got my scripts to help him with OL maps, and he provided some help in tweaking both my script and elm2pov.)

Sorry, back to the topic at hand. ;) And thanks... I'm considering an unofficial map pack download for elms that include changed guild maps like this, and tweaks to regular maps that fix decor issues that don't affect gameplay, stuff that'd be fixed in a future release anyway like bad flickering areas or harvs stuck inside rocks and such for pear hunters. On radu approval and if I ever get around to it of course. Getting it in official downloads like this is much better.

gvissers commented 2 years ago

Forget the water shader change, if it fixed anything it was purely by accident. The issue seems to be an internal texture format used in cubic filtering of the water reflection that was handled wrong by my drivers. I have just checked in a change for that. (In case anyone is wondering why that took so long to find: I thought I had cubic filtering turned off in the shader, but the client secretly adds the corresponding #define to the shader source before compiling).

pjbroad commented 2 years ago

I've done some dev build and updated the forum topic. Hopefully we can do the p1 release quite soon, perhaps encourage people to test the dev builds.

@raduprv

When we do the p1 release I presume you'll place the packages on the downloads page and update the play store version for the Android build. I'll increment the client version number and android build version when we're ready.

One of the problems we have had with the 1.9.6.0 release was that many players had not updated to the various pN release and so we were helping people with changes made years ago; this was a real pain. This time, can we update the server version for these builds; give people a while to download and install then update the server? We do not have issues with map changes this time so can give people a few days to update.

raduprv commented 2 years ago

I'd rather not do a forced update for minor bug fixes. I am sure people can update if they want to, if we blue spam it a few times. Those who don't care, whatever.

pjbroad commented 2 years ago

I'd rather not do a forced update for minor bug fixes. I am sure people can update if they want to, if we blue spam it a few times. Those who don't care, whatever.

That's a real shame as its quite a burden trying to help people that are not on the same client baseline. It was really frustrating for 1.9.6 to have people moving from such old versions; and them asking questions that were resolved so long ago. More eyes on the code help with improving it too as we've seen with this last release.

raduprv commented 2 years ago

Again, I don't think minor fixes require a forced update. Those who care about problems update the game anyway, those who don't won't. Also, an issue with server updates (but not in this particular case) is that some people somehow manage to not update their maps (or even use the old maps on purpose, and then complain about problems). Anyway, please let me know when you have the new builds ready, to upload them on the site. Also, for the Android client, can you please re-enable the scrolling through menus?

pjbroad commented 2 years ago

Anyway, please let me know when you have the new builds ready, to upload them on the site. Also, for the Android client, can you please re-enable the scrolling through menus?

Are you referring to just the options window tabs when you say menus? Unfortunately, its not a case of just re-enabling the scrolling, the code is not there. The original implementation caused issues where options would be changed while scrolling. I personally found that unbearably annoying, The current Android client uses the same code as the desktop with a few layout and spacing tweaks to be more suitable for touch. After those tweaks, I personally think using the scroll bar works fine. I'm happy to look at the full window scrolling again but it may take a while.

raduprv commented 2 years ago

Oh, I thought you said you removed them because you didn't like the accidental selection of options. From what I remember, that happened only if you tapped near a box, otherwise it worked fine. The scrollbars don't really work well, they caused a lot of missclicks for me. Maybe they can be made wider? Also, a lot of settings are there in the Android client when they shouldn't be.

gvissers commented 2 years ago

@pjbroad , I don't know if you have started packaging yet, but it would be nice if we could include #178 (new water shader for GLSL >= 1.5.0) as well. I haven't merged it yet, because it is a somewhat invasive change, but as far as I can see it seems to work well.

pjbroad commented 2 years ago

@pjbroad , I don't know if you have started packaging yet, but it would be nice if we could include #178 (new water shader for GLSL >= 1.5.0) as well. I haven't merged it yet, because it is a somewhat invasive change, but as far as I can see it seems to work well.

I'm too busy with RL to do the release currently and was kinda waiting to see if this change was ready too. If you do merge, perhaps do a squash merge so we can more easily find it again. I presume the new shader will kick in for everyone with supported drivers, not just those that do not support the older version. Is it worth adding an option to force use of the old code?

gvissers commented 2 years ago

I presume the new shader will kick in for everyone with supported drivers, not just those that do not support the older version.

Yes, I have not yet found a way to determine the minimum supported OpenGL/GLSL version. Of course I could try a shader and see what the result is (in this particular case the new drivers compile the shader all right, they just don't set certain values resulting in garbage output), like we do for the reflection texture flipping. But at some point I would like to move away from immediate mode OpenGL anyway. Might as well start the revolution small.

Is it worth adding an option to force use of the old code?

Yeah, maybe, I have not yet decided. On the one hand, we have entirely too many options already, the new shader code is very conservative in the OpenGL features it requires, is not entirely different from the old one, and AFAICT reproduces the old output exactly. On the other hand, breaking working installations is probably not a good idea either. Do you have a preference?

pjbroad commented 2 years ago

Yeah, maybe, I have not yet decided. ... Do you have a preference?

I've just tried the new code, I have an nvidia GPU and it looks like I can no-longer use water quality 2, so have no ripples. I've added a comment to the issue. This may be a driver issue, if so I would suggest we do add an option and set it as off by default for now. That way, for people that already have working shaders, they will not have issues, people like Raz can enable it and get the fix. It may also avoid needing to have the new shader files now, before we do a full release with updated data; I suggest we do another test build first.

gvissers commented 2 years ago

Very strange. If you find time, could you do me a favour and drill down in reflection.c, function get_max_supported_water_shader_quality() to see where it sets the max water shader quality to 1? I am specifically interested in whether do_disable_water_ripples near the end of the function is set to 1, or if it's because of the block above of shader tests above it.

Having said that, I should update that function to test for new shaders if those are to be used.

I'll add an option to use the ~old~ NEW shader, defaulting to ~1~ 0. EDIT: en abling new stuff makes more sense than disabling it, even if I wish everyone would update.

pjbroad commented 2 years ago

Hopefully, with the reflection changes now merged we can progress this update. First, I'll do another round of test builds for Linux, Windows and Android and post them to the forums, and if that does OK, we can do the p1. @bendoughty have you tried the new reflections code? Would you be able to build a new client for people to test before the p1 release?

feeltheburn commented 2 years ago

What's the status on this update?

pjbroad commented 2 years ago

What's the status on this update?

We're ready I think, I could do the Linux and Windows builds this weekend. @bendoughty have you been able to test the latest changes on Mac? Would you be able to do a build?

bendoughty commented 2 years ago

@pjbroad I haven't been able to test yet, RL stuff. I'll get on that now. There's an email on the way to you re: a small GL-related Mac foible I can't quite figure out.

bendoughty commented 2 years ago

Foible resolved, thank you @pjbroad. I have created a small PR (#180) with the fix. 😊

pjbroad commented 2 years ago

Thanks @ben, I've merged your changes and updated the client version information for builds. There's a new tag and release for 1.9.6.1 so I think we're ready. I'll do builds now.

pjbroad commented 2 years ago

I should add that I've produced new data and sound packages, the music pack is unchanged.

bendoughty commented 2 years ago

Thanks @ben, I've merged your changes and updated the client version information for builds. There's a new tag and release for 1.9.6.1 so I think we're ready. I'll do builds now.

Great! I'll do the macOS build now.

pjbroad commented 2 years ago

All the build are done, @raduprv we're ready to release I think. Release 1.9.6 p1

raduprv commented 2 years ago

Ok, will do the release tomorrow!

gvissers commented 2 years ago

Congrats guys, great work!

raduprv commented 2 years ago

Ok, I uploaded everything:

http://www.eternal-lands.com/forum/index.php?/topic/61733-new-client-release-1961/

Thanks everyone for the great work!

pjbroad commented 2 years ago

Al done then, closing.