OpenXcom Forum
Modding => OpenXcom Extended => OXCE Suggestions DONE => Topic started by: Stian on April 22, 2016, 09:42:11 pm
-
I've had some problems with stuttering gameplay with long view distances and noticed that it is apparently a quite common problem. Hence I decided to take a look at the way the game handles Field of View (FOV) calculations and made a significant improvement in calculation speed. You may download this new version and help test the changes below.
What does faster FOV calculations mean for you as a player:
Greatly reduced delay after each movement step for all units. This leads to smoother gameplay, faster AI turns and playable long view distances. The handling of post hit and post explosion vision calculations also benefit which leads to less delay after each shot. This in turn means faster auto fire and reduced delay whenever the terrain changes.
Download and source
Windows executable (https://drive.google.com/open?id=0B9xUQ7_2HR2VQjRiVGdQbDNKN1E) (My own additions built on Meridian's v2016-04-28) The runtime libraries normally compiled into the executable, are not, hence you need to download some additional .dll files (https://openxcom.org/forum/index.php/topic,3937.msg56803.html#msg56803) and put the ones from "deps\lib\Win32" within your game folder with the executable. Now included in Meridian's executable (https://openxcom.org/forum/index.php/topic,4187.0.html).
Source code (https://github.com/stiansel/OpenXcom/tree/oxce2.9-plus-proto-FOV)
Should you find something amiss, let me know.
PS: To get increased view distance you have to change the "maxViewDistance: 20" line in Piratez.rul.
-
Hell yes. I do experience drops in FPS myself, and this should save A LOT of CPU time, when done in a non-retarded way like this. Why the hell check at every step since tiles can only change due to attacks? I recommended this to be added to Meridian's fork once tested. Kudos for that.
The exec version stayed unchanged, so this one is compatible with the newest mod version. Or at least should... unless Windows incompatibility issues rear their ugly heads.
-
Haha shit, really?
Vision was calculated for every unit every single action? (Despite the only time it should be recalculated is after shooting/moving and only for the shooting/moving unit)
That's hilarious.
-
Not quite. It had two modes. One updating per unit and one updating around a set position on the map. The per unit is decently fast, but uses a method of fetching visible units which doesn't scale well with view distance. The around position function uses the unit one internally and triggers it for every unit within a given range of the point.
The stuttering was mostly to do with the latter function as every moved tile every unit in range had to check if it saw new units (which is fine, as otherwise you wouldn't get reaction fire or aborted movement), but since the function is also responsible for revealing tiles this lead to a lot of busywork.
-
I think that you mess up line ending in your commit. GitHub should display only lines you change not big diff containing whole function.
This hide most important changes you made in it.
I planed at some point look at this functions too, but as you do hard work I will gladly pull it to my branch (Meridian will probably do the same).
-
Oops, you're quite right. Mixed windows and linux line endings. I reverted the latest commit and uploaded it again with slightly better consistency. BTW: Would it be better if I recreated the branch with all changes in one commit?
-
At least for me will be a bit easier to cherry-pick it. Overall if all commits have one goal and touch only one thing then probably better if it is only one commit.
If you did some heavy refactoring that touch lot of other files then would be better to move it to separate commit, other that one implementing main functionality.
-
I'm planning some additional changes to how and when the FOV calculation is used. Here's a rundown for movement-related FOV calculations:
For every step of movement:
what it was:
- Every unit in range of the moving unit clears and updates their visible units.
- All player units in range clears and updates their visible tiles.
what the new version does:
- Every unit in range of the moving unit clears and updates their visible units (in a faster manner).
- Only the moving character clears and updates its tiles (if a player unit).
What I'm planning to change next:
- Only units which saw the moving unit before its move needs to check for new visible units. (In case the moving unit blocked sight to someone behind it.)
- Every unit in range needs to check if it can see the moving unit after the step.
- Only the moving character clears and updates its tiles (if a player unit).
So thats my todo-list for movement-related FOV calculations. I may also update the projectile hit code for explosive projectiles to use the optimizations I added for the non-explosive ones (basically don't recalculate the entire maps' worth of sun and other lighting, as well as the FOV of all units in range, unless something has actually changed as a result of the hit)
I see there's a bit of back and forth in my commits, so I'll make a new branch when I've updated the source with the new features.
-
So thats my todo-list for movement-related FOV calculations. I may also update the projectile hit code for explosive projectiles to use the optimizations I added for the non-explosive ones (basically don't recalculate the entire maps' worth of sun and other lighting, as well as the FOV of all units in range, unless something has actually changed as a result of the hit)
I see there's a bit of back and forth in my commits, so I'll make a new branch when I've updated the source with the new features.
how performance look after your changes? How much far you can now push visibility to see problems?
I recently change some lighting handling in my branch: https://github.com/Yankes/OpenXcom/commit/10fd826aed10cdf3b89d3b25599ce357bc1a26b2
It had some low hanging fruit improvements.
-
I haven't noticed any fps drops or lagging animations with this .exe in the few missions I've played, and no clear bugs either. So good job so far. :)
-
Would it be possible to update the .exe with Meridian's latest additions? I'd appreciate it a lot, playing without the movement stutter on maxviewdistance: 30 is very enjoyable, but some of Meridian's bug fixes and new features are also pretty tempting. :D In any case, thank you a lot for making this.
-
Glad you're still finding it useful! :)
I should have a new version with greatly improved performance for you fairly soon. I just need to update a few more places where the new FOV calculation is initiated, then run a couple more battles to look for the most obvious bugs.
how performance look after your changes? How much far you can now push visibility to see problems?
Attached is a graph of my latest test on movement-related calculations. The "original" bars is the current OXCE build. The newFOV2 bars is my current build. (not the executable from the original post). The graph tracks a single unit running to a craft door, then a second unit running into the craft and opening a second door. Each step along the X-axis is one step for the unit with a FOV calculation for every unit in range (34 units in range total). The Y-axis plots the time used per step for both algorithms. The view distance was set to 30. The median time used per calculation was 58.41ms and 5.782ms. The two slow newFOV2 bars are the doors opening, as that part of the code hasn't been updated to use the new features yet, which causes the algorithm to default to a semi-original behaviour.
I've tried playing with view distances up to 60, but the reactionfire gets really crazy when everyone can see across the map....
-
Now that IS a huge performance improvement :o
-
Reaction shoots will be "fixed" in next Extended version (this is already implemented). You will have full control when units will react.
One way to fix doors performance could be options to add some kind of "dirty rect", in each update only LOS that cross it will be calculated for each unit.
This could be used with explosions too. If you pass rect that contains whole map you will get current behavior, if empty nothing will be updated.
-
That's actually pretty close to how I'm updating FOV at the moment. I imagine a circle around the event (with some clearance) then place lines tangent to the circle as viewed from the unit and update units and/or tiles within that area. For tiles it's a little more complicated due to the nature of the bresenham lines used to reveal terrain, but it appears to work well now.
As for the doors I just haven't updated the function call from the code handling the doors yet, as I've been busy testing the basic functionality. With it using the old function call convention certain parameters are set to default values causing the algorithm to ignore the arc handling part of the update. Should be an easy fix as long as I remember that doors can have different sizes and scale the event accordingly.
-
Great, I will looking forward for final version.
-
Great, I will looking forward for final version.
Me too.
-
Updated the first post with the new source code and an executable built on top of Meridian's v2016-04-28.
Should I make some kind of illustration explaining how it works?
-
Yay, now I can upgrade to 98E. Thank you very much Stian. :)
-
Discovered a bug with doors opening. (so much for the easy part, right?) :) If an AI opens a craft door your units will not reveal tiles within the craft. They will still reactionfire at units standing on the tiles which should've been revealed though. I know when it bugs out and have a dirty fix for it, but I'd rather try to fix it properly so that units don't slow down on doors. Sorry about this.
Edit: ...and I know exactly what I did wrong. I did a last minute fix of a bug relating to explosions at the edge of unit visibility. That is the centre of the explotion was outside of range, but the effect wasn't. This fix had a small oversight (forgot to square the range...) and stopped most positionupdates from working properly. Executable will be updated shortly, just need to remove the debug info.
Edit2: Updated the executable and the source.
-
I've cherry-picked the two commits, but it doesn't compile for me...
-
Ah, change from
(*j)->getVisibleTiles()->clear(); to (*j)->clearVisibleTiles(); (My Git upload tool hates me.)
-
Just wanted to thank everyone involved in this, the gain of performance is incredible. It feels like my computer had a major hardware upgrade. I am using a viewrange of 40 fwiw.
-
There was also discussion about 2 further possible upgrades to performance:
1. Why the game is so slow on high-res? (I can play OK in 480px, even 640px, but my native res - 1680px - slows the game down to a crawl, even with the new FOV calculations).
2. Why the pre-battle slows down so much if you have much stuff on your craft?
Investigating these 2 strange cases might bring us even more speed... :)
-
Just wanted to thank everyone involved in this, the gain of performance is incredible. It feels like my computer had a major hardware upgrade. I am using a viewrange of 40 fwiw.
I'm so glad that it is working and providing such a benefit for people! Thanks. :)
There was also discussion about 2 further possible upgrades to performance:
1. Why the game is so slow on high-res? (I can play OK in 480px, even 640px, but my native res - 1680px - slows the game down to a crawl, even with the new FOV calculations).
I'm getting this if I have OpenGL turned on. Basicly if I turn on an OpenGL filter my CPU immediately jumps to full load on that core regardless of resolution. With any of the non-OpenGL filters 1920x1080 is working well for me. So something is obviously wrong with the hardware acceleration code, or device selection, as it shouldn't be slower than software.
2. Why the pre-battle slows down so much if you have much stuff on your craft?
I had a look at this earlier in the week. It appears to be caused by the inventory packing code. Sadly I've managed to overwrite my basedefence savegame where this issue was most prevalent for me, and using the battle generator doesn't give me the same slowdown that save had for some reason. Do you happen to have a save handy?
-
If we are still using the same inventory code as the original game i'm not surprised. It had a hard limit for number of items in loadout screen to prevent memory issues. To my recollection openxcom just took off the limit. Most the time you would never notice the bad code since modern machines have orders of magnitude more memory.
-
I had a look at this earlier in the week. It appears to be caused by the inventory packing code. Sadly I've managed to overwrite my basedefence savegame where this issue was most prevalent for me, and using the battle generator doesn't give me the same slowdown that save had for some reason. Do you happen to have a save handy?
I have a good save.
Will upload later today... I haven't looked at it yet, but I feel it must be the arrangeGround() method... probably arranging each item separately instead of arranging stacks, so 1000+ credit chips on the ground will give it a hard time. Just guessing tho.
-
Yes, I seem to remember that's pretty much how it went. Start from the beginning for each item, add one to the stack, etc. A quick fix would be to add as many as possible each step. A good bit of the extra busywork could also be reduced if it kept track of free slots, just like those imagestacking algorithms do, rather than scan through the inventory with each new item. I'll take a look later.
-
Save attached (0.98e, no other mods)
-
With that save there are 2563 items to be processed into 185 stacks. Turning the inventory page takes 621ms with the current algorithm on my machine. I introduced some changes, and while I still need to clean up the code a bit, it's looking good so far with pages turning in 8ms. With pre-sorting of items from large to small it completes in 5ms, but that also alters the display order.
-
With that save there are 2563 items to be processed into 185 stacks. Turning the inventory page takes 621ms with the current algorithm on my machine. I introduced some changes, and while I still need to clean up the code a bit, it's looking good so far with pages turning in 8ms. With pre-sorting of items from large to small it completes in 5ms, but that also alters the display order.
I for one, would probably not be bothered by altering the display order. Right now, its so freaking chaotic, that going Large to small would really help.
I mean, yeah, its nice for gauss to be grouped together, but put 7 pages worth of inventory on board, and now try to find those smoke grenades. :?
This is why I'd also love to have that "Sort Items" in inventory ability. A dropdown from the top, where you could choose "Grenades" "Pistols" "Rifles" "Infiltration" etc. :)
-
With that save there are 2563 items to be processed into 185 stacks. Turning the inventory page takes 621ms with the current algorithm on my machine. I introduced some changes, and while I still need to clean up the code a bit, it's looking good so far with pages turning in 8ms. With pre-sorting of items from large to small it completes in 5ms, but that also alters the display order.
I vote for not changing the order.
8ms and 5ms is no difference... and this way it has a better chance to get upstream into vanilla.
This is why I'd also love to have that "Sort Items" in inventory ability. A dropdown from the top, where you could choose "Grenades" "Pistols" "Rifles" "Infiltration" etc. :)
Already on my todo list.
-
Displaying items by categories in the inventory will be cool!
As for sorting, large to small isn't too bad but that's not much to go from. There's a whole lot of 3x2, 3x1 and 1x1 items, Is it done by listOrder currently? That's what I would prefer to see. Then it groups all your melee weapons first, then guns, sort of by damage/tech level, then equipment.
I don't really care for "maximum packing", ie filling all the squares at the beginning. In fact, it tends to make things really hard to find since their positions depend on what else you have packed. Personally, ordering by list order would be best. That keeps clips next to their guns and things sorted in the way we are used to from the items lists.
-
Sorting by size AKA "you will never find ammo for your weapon". No, please, I'd rather not :) The performance gain is already impressive.
-
I uploaded the source on Github (https://github.com/stiansel/OpenXcom/tree/oxce2.9-plus-proto-inventory) and as a pull request to oxce2.9-plus-proto. I retained the item order. Without the debug info it ended up with stacking Meridian's test save inventory in 5.2ms. Hopefully that should give enough room, even when we account for larger item collections.
This is why I'd also love to have that "Sort Items" in inventory ability. A dropdown from the top, where you could choose "Grenades" "Pistols" "Rifles" "Infiltration" etc. :)
Already on my todo list.
I changed it so that it should be easier to filter items of interest. If you remove any items you do not wish to display from the "itemListOrder" vector on about line 1066 it shouldn't show up on the display. Hope that helps.
-
A bug report on the inventory stacker: https://openxcom.org/forum/index.php/topic,4187.msg64490.html#msg64490
-
Thanks for the notification Meridian. Uploaded the fix.
-
Here's an animation of my latest inventory experiment (may have to click the image to animate). A search field for those really big inventory lists.
It currently allows searching by the (localized) name of items. There's still some work to do, like allowing damage types to be searched for, and hiding artifacts, as well as some useability improvements. Shouldn't take all too long though.
-
That looks excellent. I often think about that kind of search filter, but I didn't think anyone would take the time to implement it!
The only bad thing is that if the loadout screen has a search filter, I'm going to be wishing for a search filter everywhere else as well! In particular, it would sometimes be really handy to search the Vaults and Fence screens (although, I'm getting to the point where I've memorised where the most important things are anyway...)
-
Here's an animation of my latest inventory experiment (may have to click the image to animate). A search field for those really big inventory lists.
It currently allows searching by the (localized) name of items. There's still some work to do, like allowing damage types to be searched for, and hiding artifacts, as well as some useability improvements. Shouldn't take all too long though.
This looks very good indeed.
Search box component could be used on many other places too.
A few comments/thoughts:
- searchbox should be probably on a different place than the current tooltip, so that they don't overlap... maybe put a small searchbox where the tooltip is now and move the tooltip slightly more to the right... or somehow show one or the other, depending on situation?
- if you implement a search box component, it would be nice if it had a nice UI, for example a border, background color, maybe a watermark ("search here...") and a small "x" on the right side to clear the content... I'm thinking something with a size/height of a button/combobox... too bad there is really barely any space left on the inventory screen... it won't be easy to fit it in nicely (unless you already solved that :)
Anyway, just thinking out loud, looking forward to merging your feature(s) into OXCE+ ;-)
-
Yeah, space is really at a premium on that screen. I thought of placing it to the side, but some items have really long names and may overlap. I considered it as a seperate button, but a pop-up would require more clicks and may not allow the user to manipulate the inventory while searching. That search component idea with a wrapping box and an x to clear sounds pretty good though, so I may have to rethink the whole placement.
The current implementation shares the location with the item/button tooltip. If the user hovers over the tooltip it changes to a "click to search" string. If clicked it disables the item/button tooltips while input is active. The filter updates live as in the animation, but if the user presses enter the filter is saved and tooltip functionality is returned. I'm thinking of adding a filter active prompt to the tooltip whenever the tooltip is empty to reduce confusion ("didn't I have more items than this!?").
Technical implementationwise it's not a component (yet atleast), I'm simply using the TextEdit box with it slightly changed to allow exiting with zero-length strings. Should be room for something similar in the upper left of the workshop screen, right? :)
-
Should be room for something similar in the upper left of the workshop screen, right? :)
There is now... but I want to add another filter there with more options:
- Default
- Default (supplies ok)
- Default (no supplies)
- New
- Facility required
- Disabled
- Everything
And also change the button on bottom left to a combobox with more actions:
->Mark all as new
->Mark all as seen
->Mark all as disabled
->Mark all as enabled
Also right-click would change item state state from new -> normal (enabled) -> disabled -> new.
But with a search box... this might all change, we'll see :P
-
I don't really know if it is a good idea to make a reminder here (if so, I'm really sorry for being offtopic) but you, guys, are talking about the interface dimensions problems and maybe you haven't seen that: https://openxcom.org/forum/index.php/topic,1558.0.html. xracer was working on a higher resolution version of the OXC interface but as far as I can see looking at the GitHub commits at the moment he doesn't have time to work on his project. If someone could finish the project I think this would give a lot of new possibilities to extend the interface and integrate new features.
-
I think this would give a lot of new possibilities to extend the interface and integrate new features.
That experiment have a one little problem that become to really a headache. Different monitors with different screen resolutions and PPI. Solution requires a correct design from the start of experiment that will works with most resolutions without readaptation.
-
Cool stuff!
My opinion is this:
Search box in the equip screen = cool gadget
Search box in vaults/store/workshop = lifesaver :)
-
@stiansel:
Yankes cherry-picked your changes to OXCE branch: https://github.com/Yankes/OpenXcom/commits/OpenXcomExtended
He also changed/fixed a few things:
1. https://github.com/Yankes/OpenXcom/commit/ad01723a2ccd611c0ef2447a095defe9f2a3027c
2. https://github.com/Yankes/OpenXcom/commit/986e534286c42a957615fbb0f8a8d703045c96ff
Should I do the same changes too?
-
Yeah, those changes look good to me.
Though there's one thing that needs to be checked first. The FOV calculation needs the supplied radius to be large enough to fully cover the event, so it may need a +1 to the radius.
Edit: I wouldn't add the 3D distance fix as it will produce different results depending on whether a position based update or a unit based update takes place. That addition fix in the same commit is very much recommended though.
-
I updated the repository for the item stacker. I changed it a bit to better facilitate filtering by category. Namely I've changed it to stack each item in advance in its stackable subtypes, rather than bunch all items of the same type, then later figure out the stackable subtypes. This makes filtering out a specific subtype easier later on. (actual filtering not included as it's probably better to add that to a different branch?)
Also noticed a getItem function which (worst case) scanned the entire item inventory for each tested location of the stacker. With the other changes I've done to the stacker it tries to find room for stacks rather than individual items. So with the stacker no longer needing to know which item is present, just that something is there, I changed it to a small occupancy lookup table instead. The new version is about 10 times faster on the previous test save you gave me (and scales better).
-
Edit: I wouldn't add the 3D distance fix as it will produce different results depending on whether a position based update or a unit based update takes place. That addition fix in the same commit is very much recommended though.
OK, I'll take the addition fix and change the distance calculation back to 2D.
-
I updated the FOV repository with a new bug fix. While investigating why Yankes hadn't noticed a difference with 3D distance set for the position based FOV updates I found a few cases where I had been inconsistent in my use of 2D and 3D distance. After this fix visibility near max range should be consistent with the base game behaviour again.
-
@Stian:
Could you have a look at this? Adding the quick filter messed it up a little bit... but can't reproduce reliably. Maybe you'll spot an issue right away...
https://openxcom.org/forum/index.php/topic,4520.msg65983.html#msg65983
-
Does the issue also occur if you change the search term function to use arrangeGround(false)? (I don't have access to a compiler right now and probably can't take a much closer look for a couple of days.)
Edit: The extra spaces may be caused by incorrect values in the start index cache. Try disabling the startCache stuff by simply setting x=0. With the other changes done the performance loss is fairly small. If this turns out to be the culprit I'll fix it when I get back to a more useful computer.
-
I certainly prefer this bug over performance loss. It happens very rarely and basically only if you click the page change button like crazy.
-
Found and corrected the inconsistent inventory spacing bug as reported by Arthanor and Meridian. It was caused by a read outside of the assigned cache, hence the random nature of the error. This could also cause a CTD on opening of the inventory, so if anyone was experiencing that, it should be fixed too.
https://github.com/stiansel/OpenXcom/commit/6c6b4f17acb121ca4869121858ff538c49851d50 (https://github.com/stiansel/OpenXcom/commit/6c6b4f17acb121ca4869121858ff538c49851d50)