Improvements and some bug fixes, will someone merge?

Game Talk The Lab
1

I’ve accumulated quite a few changes now, will someone merge this code to official build?

Windows binary:
(built on latest source from github)

BUG FIXES:

  • optimised rendering, significant speed increase and much smoother game-play
  • flashlight to draw “gradual fade off” properly all around instead if only on one side
  • animations now working on Windows, sleeping and waiting “fast forward”, as seen on Linux

NEW FEATURE:

  • automatic “distance view” when driving, shifts view forward in front of the vehicle (depending on current speed and bearing), so you can see more road in the direction car is facing

CHANGES:

  • instead of grey “#” character darkness is completely black
  • vehicle headlights have increased range
  • few colors got brighter, and possibly other minor visual changes

There are many changes since 0.2 version, this above list is just what I did and what is not on github.

2

What’s the address of your github?

3

I don’t have a github. Would that make it easier? So you probably want to merge optimisations and bug fixes, possibly not color changes and such, but what about automatic distance view when driving, have you tried it, what do you think about it?

4

We’ve got no way to look at the code, and since most of the devs are on linux, no way to even try out your changes.

So yes, get a github and push to it so we can check it out.

5

If you are interested to have proper rendering and animations on Windows then you will find Windows computer to try it out. There is no other way to see those changes since they are specific for Windows build. And source code I can copy/paste here or upload wherever, and explain what, why, and how if that’s what you would like to know. In any case I don’t see any reason why would I need to set up my own github for a few changes. Why github anyway, would it make any easier to merge the changes?

6

Yes. It would allow you to fork and upload your code and make a “pull request”, making it trivial for the team to review your code and merge it in, and giving us the ability to comment on specific lines if there are questions or concerns, for you to make future changes to the code if needed to address those concerns without losing history, for us to push it into a side branch of our own if we want to tweak things before merging, etc and so on.

Obviously, you are correct about the windows bit - those of us not on windows wouldn’t be able to test out the windows-specific components, but most of what you described isn’t just windows specific, and we would at least be able to see if the code looked good, and limit the amount of review the windows folks with their limited numbers and time would have to devote to going over it.

At the very least you should post a diff.

7

I don’t see how github makes anything trivial. If the code is in separate branch than any recent change relating to the same code as my changes can cause incompatibility, or worse, it could be a source of sneaky non-obvious bugs. It needs to be done by hand if you want to be certain it still fits with the most recent official build at the time of the merge. Anyway, if you already have room to put this in some side-branch in the meantime then go ahead and do that.

SOURCE:

There is “//CAT:” comment before any change.

Obviously, you are correct about the windows bit - those of us not on windows wouldn't be able to test out the windows-specific components, but most of what you described isn't just windows specific, and we would at least be able to see if the code looked good, and limit the amount of review the windows folks with their limited numbers and time would have to devote to going over it.

At the very least you should post a diff.

The most important change is about Windows proper rendering, speed up and working animations. Although I think Linux version runs smoother and faster too, faster than Windows certainly. The reset are just some minor bug fixes and there is also this automatic distance viewing feature for driving.

BUG FIXES:

  • optimised rendering, significant speed increase and much smoother game-play
  • animations now working on Windows, sleeping and waiting “fast forward”, as seen on Linux

The main problem was in catacurse.cpp, in getch() function. InvalidateRect() does not belong there, so I moved it to DrawWindow() and added call to UpdateWindow(), like this:

void DrawWindow(WINDOW *win)
{
.
.

    win->draw=false;                //We drew the window, mark it as so

//CAT:
  InvalidateRect(WindowHandle,NULL,true);
  UpdateWindow(WindowHandle);
}

I also removed all the unnecessary stuff from getch() function, so it boils down to this:

[code]int getch(void)
{
//CAT:
refresh();

lastchar=ERR;
    do{
        CheckMessages();
    }while (lastchar==ERR);

return lastchar;

};
[/code]

That will give you working animation on Windows, only then you will see a lot of flickering, so the second part of this fix mostly involves commenting out redundant calls to wrefresh(), refresh(), refresh_all(), and such, in game.cpp, and moving some of them around or elsewhere. That would fix the flickering and speeds up rendering quite a bit. And that’s it. Actually I also disabled calls to nanosleep(). There is no need to slow down what is already slow and needs to be faster, it only makes for choppy scrolling. These changes will also fix more recent bug where there is no message shown on top of the screen asking which direction you want to examine, and some other action related questions like that are missing.

BUG FIXES:

  • flashlight to draw “gradual fade off” properly all around instead if only on one side

Basically commenting this part out in map.cpp, map::draw() function will fill the bug:

//CAT:
/*
   // Don't display area as shadowy if it's outside and illuminated by natural light
   else if (dist <= g->u.sight_range(g->natural_light_level())) 
   {
       lowlight_sight_range = std::max(g_light_level, natural_sight_range);
   } 
*/

I see someone just very recently also tried to address this issue, but their result does not have dark shade area. Is that what what you call “gradual_night_light”? What is it supposed to be difference with that option ON and OFF, can you show me on a screenshoot? In any case, I also reduced that dark shade rim to be one pixel wide instead of half of the total light range, because it grays out yellow lines on the road so it often looks as if the road ends when it does not. And those yellow lines should be visible even further and more than light range, certainly not less visible than the pavement itself.

NEW FEATURE:

  • automatic “distance view” when driving, shifts view forward in front of the vehicle (depending on current speed and bearing), so you can see more road in the direction car is facing

In game.cpp add this few lines at the end of game::pldrive() function:

//CAT:
	float cat_vel= veh->velocity*1.5;
	if(cat_vel > 10000) cat_vel= 10000;
	float cat_sin = sin(veh->turn_dir*M_PI/180)* cat_vel;
	float cat_cos = cos(veh->turn_dir*M_PI/180)* cat_vel;

		u.view_offset_y= (int)cat_sin/1000;
		u.view_offset_x= (int)cat_cos/1000;

In map.cpp, map::unboard_vehicle() function add these lines to center the screen in case player bailed from a moving vehicle:

//CAT:
 g->u.view_offset_y= 0;
 g->u.view_offset_x= 0;

And to increase headlights range, change this line in lightmap.cpp, light_map::generate() function:

//CAT: longer range headlights
//     float luminance = c[sx - x + LIGHTMAP_RANGE_X][sy - y + LIGHTMAP_RANGE_Y].veh_light;
	 float luminance = 250+ c[sx - x + LIGHTMAP_RANGE_X][sy - y + LIGHTMAP_RANGE_Y].veh_light;

That’s about it.

8

[quote=“LazyCat, post:7, topic:406”]And to increase headlights range, change this line in lightmap.cpp, light_map::generate() function:

//CAT: longer range headlights // float luminance = c[sx - x + LIGHTMAP_RANGE_X][sy - y + LIGHTMAP_RANGE_Y].veh_light; float luminance = 250+ c[sx - x + LIGHTMAP_RANGE_X][sy - y + LIGHTMAP_RANGE_Y].veh_light; [/quote]

This is me being nitpicky but the lights take their brightness from the vehicle part:
[tt]c[px][py].veh_light = vehs[v].v->part_info§.power;[/tt]

So it would be cleaner just to increase the power stat on headlights and have the same effect without the constant.

Changing line 283 in veh_type.h from this:

{ "head light",        '*', c_white,  '*', c_white,  10, 20, 120, 0, itm_flashlight, 1,

To this, should have the same effect of your +250 increase.

{ "head light",        '*', c_white,  '*', c_white,  10, 20, 270, 0, itm_flashlight, 1,
9

[quote=“Shades, post:8, topic:406”]Changing line 283 in veh_type.h from this:

{ "head light",        '*', c_white,  '*', c_white,  10, 20, 120, 0, itm_flashlight, 1,

To this, should have the same effect of your +250 increase.

{ "head light", '*', c_white, '*', c_white, 10, 20, 270, 0, itm_flashlight, 1, [/quote]

Thanks. You are right to point it out and who knows better shouldn’t be submitting hacks like that.

Is there some similar piece of code that defines the range for the flashlight?

10

Unfortunately I never got around to adding lights as a tag to items themselves, them, along with critters that generate light sources are hard coded.

Yet another thing that has been on my todo list for months…

11

Oh my god! You have to merge this! YOU HAVE TO.

12

LazyCat, while we certainly appreciate the effort and code improvements, I get the feeling you haven’t done things like this too often. It’s okay, but in the future, I would stress that you should at LEAST post a diff for us. As someone who has to do this sort of thing for a living, merging and managing and combining code and oh god all the day every day, you might not even believe how much easier it makes it on everyone but it totally, totally does, and it’s relatively easy to do (unlike setting up your own branch, which admittedly can be a bit hard).

Shades, I don’t suppose you could turn this into a pull request for us, since you already looked over everything? Bug fixes at at least are almost certainly going to be mainlined.

13

[quote=“GlyphGryph, post:12, topic:406”]LazyCat, while we certainly appreciate the effort and code improvements, I get the feeling you haven’t done things like this too often. It’s okay, but in the future, I would stress that you should at LEAST post a diff for us. As someone who has to do this sort of thing for a living, merging and managing and combining code and oh god all the day every day, you might not even believe how much easier it makes it on everyone but it totally, totally does, and it’s relatively easy to do (unlike setting up your own branch, which admittedly can be a bit hard).

Shades, I don’t suppose you could turn this into a pull request for us, since you already looked over everything? Bug fixes at at least are almost certainly going to be mainlined.[/quote]

I want to sort it all out right here, sooner rather than later. I want to know what changes are good, and if not, why not. I expected more talk about the binary itself, what it actually does, rather than how code looks like. There is nothing much to talk about the code, 90% of my changes are simply commenting out redundant stuff, but still I think it’s much easier to understand and merge these changes by reading my post above than by looking at diffs.

Basically I wanted more direct feedback, to hear how much better it is for Windows users now, whether animation is maybe too fast, or should it be even faster. I’d like to hear how people like auto shift of viewing distance when driving, and what’s the opinion on completely black darkness tiles instead of gray “#” symbols. I think these are things to talk about first rather than leave the decision to one person and just merge it or not.

14

My thoughts as a windows player:

It looks and feels better overall. Moving around and, particularly, driving has been extremely enhanced to the extent that I have to restrain myself to not keep the 5 key pressed, to avoid crashes. Now it really feels like driving a vehicle.

The black tiles for darkness really add to the immersion, and the fast-forward is a nice touch.

Those are my thoughts after playing with this build for a bit. I have yet to try other features properly, such as headlights and flashlights.

Bugs:
The y/n prompts aren’t cleared when grabbing something and the menu listing available objects is displayed. They only disappear after you close the items menu, which can confuse players if they aren’t aware of that issue.

As I said, I’ve barely played with it, but so far it looks great. I hope you guys get to sort out this, erm, communication problem, because, really, this looks like a nice addition.

15

It’s great, it really is, I enjoyed the smoothness of driving a vehicle greatly and all other changes are great, too. Except the font. The font hurts my eyes.

16

[quote=“kenoxite, post:14, topic:406”]My thoughts as a windows player:

It looks and feels better overall. Moving around and, particularly, driving has been extremely enhanced to the extent that I have to restrain myself to not keep the 5 key pressed, to avoid crashes. Now it really feels like driving a vehicle.

The black tiles for darkness really add to the immersion, and the fast-forward is a nice touch.

Those are my thoughts after playing with this build for a bit. I have yet to try other features properly, such as headlights and flashlights.

Bugs:
The y/n prompts aren’t cleared when grabbing something and the menu listing available objects is displayed. They only disappear after you close the items menu, which can confuse players if they aren’t aware of that issue.

As I said, I’ve barely played with it, but so far it looks great. I hope you guys get to sort out this, erm, communication problem, because, really, this looks like a nice addition.[/quote]

Well I am glad to hear that, it makes the effort more worthwhile. I can’t reproduce that bug with y/n prompts, how do you get to that menu, are you picking up an item, multiple items, something in particular?

17

It happened any time the y/n prompt appeared, such as when filling a container from a toilet.

18

Are you talking about that I set up the font to be square? I guess it may look squashed if you are used to 8x16 that is default in official build. Or do you get some completely different font than usual? Beside setting the font to 16x16 I commented out “#Terminus” line in FONTDATA file, so it can be resized, and then I think the program uses a font given by the operating system according to who knows what settings.

Font should look like this here:

If not, then you should pick your own font and change FONTDATA so the game uses that particular font. In case you are just used to vertically stretched 8x16 appearance of the same font simply modify FONTDATA file back to 8x16 values, although I highly recommend using square fonts so the main graphical display does not get distorted and then game looks much better in my opinion, plus it doesn’t seem you move faster when moving vertically.

19

Now that I think of it, maybe the issue I reported has to do with the font. I changed the one provided with another one from another build (an 8x16 one). I have to check that later.

I’ll edit this post whenever I do.

20

Yes, I was talking about the font’s squareness. I don’t know why, but for some reason it’s harder for me to read and look at.