CitiesSkylinesMods / TMPE

Cities: Skylines Traffic Manager: President Edition
https://steamcommunity.com/sharedfiles/filedetails/?id=1637663252
MIT License
569 stars 85 forks source link

Allow separate control of parking lanes on 1-way streets. #516

Open originalfoo opened 5 years ago

originalfoo commented 5 years ago

On one-way streets, only a single parking icon appears which affects parking on both sides of street.

Would it be possible to get more granular control so each parking lane can be toggled independently?

EDIT: We could also support parking for lanes with m_direction=both . see https://github.com/CitiesSkylinesMods/TMPE/issues/516#issuecomment-587837970 this has to be removed after fix : https://github.com/CitiesSkylinesMods/TMPE/wiki/Notes-for-Asset-Creators#parking-lanes

kianzarrin commented 4 years ago

The back end interface sets/unsets parking based on direction. fixing this requires fixing the interface and making the old interface obsolete. the interface should accept near/far side?

kianzarrin commented 4 years ago

According to the parking code I think that both sides of the street are park-able.

                  // randomize target position to allow for opposite road-side parking
                    ParkingAI parkingAiConf = GlobalConfig.Instance.ParkingAI;
                    segCenter.x +=
                        Singleton<SimulationManager>.instance.m_randomizer.Int32(
                            parkingAiConf.ParkingSpacePositionRand) -
                        (parkingAiConf.ParkingSpacePositionRand / 2u);

                    segCenter.z +=
                        Singleton<SimulationManager>.instance.m_randomizer.Int32(
                            parkingAiConf.ParkingSpacePositionRand) -
                        (parkingAiConf.ParkingSpacePositionRand / 2u);

                    if (netManager.m_segments.m_buffer[segmentId].GetClosestLanePosition(
                        segCenter,
                        NetInfo.LaneType.Parking,
                        VehicleInfo.VehicleType.Car,
                        out Vector3 innerParkPos,
                        out uint laneId,
                        out int laneIndex,
                        out _)){
                       NetInfo.Lane laneInfo = segmentInfo.m_lanes[laneIndex];
                        if (!Options.parkingRestrictionsEnabled ||
                            ParkingRestrictionsManager.Instance.IsParkingAllowed(
                                segmentId,
                                laneInfo.m_finalDirection))
originalfoo commented 4 years ago

Basically there should be an icon for each parking lane on a road segment

Similar to how Speed Limits tool works when in "lane-wise" mode.

So if a road has 3 parking lanes, even if it's one-way road or whatever, there would be 3 parking icons, one for each parking lane.

kianzarrin commented 4 years ago

@aubergine10 This requires a MAJOR change to back end in a way that it may not be backward compatible.

The current backend uses Is/SetParkingAllowed(ushort segmentID, NetInfo.Direction direction) The new backed should be ISSetParkingAllowed(uhort segmentID, byte laneIndex)

the serialization format is:

        [Serializable]
        public class ParkingRestriction {
            public ushort segmentId;
            public bool forwardParkingAllowed = true;
            public bool backwardParkingAllowed = true;
        }

Backward compatibility is tricky here. I should check version and load based on version because XML stores based on variable names the new format should be:

        [Serializable]
        public class ParkingRestriction {
            public ushort segmentId;
            bool []allowed ; // first second third ...
        }

EDIT: with also may need a ConvertIndex(segmentId, laneIndex)// convert lane index to first, second third ... to reduce the size of the array above.

Do you agree with this?

originalfoo commented 4 years ago

Ah, I hand't considered that it would require changes to the data persisted in savegame.

There's still lots of people moving over from v10 and then sometimes try reverting back to v10 if they run in to problems. Should we delay making changes to a later version?

kianzarrin commented 4 years ago

I can provide backward compatibility. But this issue in whole should be handled separately than #702 . Its just too many things at once.

What should the interface look like though? should we store parking information for each laneindex? or only the ones that are marked as parking? (its a difference between speed VS memory)

EDIT: we could have a 32bit flag variable for each landIndex so its not much of a memory. The interface could be:

Is/SetParkingAllowed(uhort segmentID, byte laneIndex) // laneIndex is the index of lane in `NetInfo.m_lanes[]` 

This way there is no need to have a loop to go through all lanes to see which one is a parking lane.

originalfoo commented 4 years ago

What happens if some lunatic creates a road that has > 32 lanes?

(I've no idea if there's a limit on the number of lanes - what size is the integer used by prefabs to index lanes?)

krzychu124 commented 4 years ago

There is no limit for lanes and number of lanes is always calculated on demand (not cached/ hardcoded)

kianzarrin commented 4 years ago

@aubergine10 @krzychu124 LaneIndex is byte size.

What happens if some lunatic creates a road that has > 32 lanes?

then the parking will not work lol! and users will not be surprised! it will work though because usually the parking lanes are in the first 4 lanes ( the first 2 are pedestrian). but that's not always the case. If I want to go in a loop for a 32 lane in every simulation step that is not going to look good. its better if the parking didn't work in crazy case rather than the game running slowly. the flag can be 64 bit.

originalfoo commented 4 years ago

What happens if some lunatic creates a road that has > 32 lanes? then the parking will not work lol!

Dude, you just banned parking in all of China lolol How will they make 50-lane toll booths that exit in to just 4 lanes?

image

kianzarrin commented 4 years ago

OK an array of 64 bit flags then how is that @aubergine10 . the loop that way is going to be small for the first 64 lanes :).

originalfoo commented 4 years ago

I was joking lol.

One thing to check before implementing the 32-bit flag thing is how will it affect pathfinder? Does it need to factor in which side of road the vehicle is on as to which side it looks for parking on? Depends what info it already has at that point (like lane index and segment id?) as to what the impacts could be.

kianzarrin commented 4 years ago

@aubergine10 lol! It works. lane index is available to path/parking finder as you can see in the code i provided in https://github.com/krzychu124/Cities-Skylines-Traffic-Manager-President-Edition/issues/516#issuecomment-586526219. Speaking of the witch GetClosestLanePosition is really slow for this part of the code. remember when you were complaining it reduced FPS when I used this to improve precision of vertical hitpoint in the lane connector?

kianzarrin commented 4 years ago

Screenshot (554)

some UL roads are acting weird see https://github.com/CitiesSkylinesMods/TMPE/pull/708#issuecomment-587599978 and https://github.com/CitiesSkylinesMods/TMPE/pull/708#issuecomment-587821309

the parking lanes has direction=both: Screenshot (559)

EDIT: Banning parking on this side does nothing: Screenshot (553)

banning parking on this side bans is for both sides. Screenshot (556)

originalfoo commented 4 years ago

This is almost certainly root cause of #366 as well.

What value should the setting have? It's something that I should note in our guide to asset authors.

kianzarrin commented 4 years ago

@aubergine10 What value should the setting have? It's something that I should note in our guide to asset authors.

Forward or backward. matching the nearby lane. both also is not too bad i guess. We can modify TMPE to support it. what do you think?

originalfoo commented 4 years ago

I'd like to provide support for both, but a larger part of my brain is saying not to...

For example, in #366 I noticed that vehicles were doing 180 degree turn to park 'facing wrong way' in adjacent parking lane. And another user noted that cars were facing same direction on both sides of road. So from end-user perspective at least, there is the issue in which way the vehicles are facing when they park.

Also, I think it would still be cool - for one or two lane roads where m_canCrossLanes = true - if vehicles could park "on the other side" (ie. do 180 degree turn to park on other side of road). To facilitate that, we'd have to have correct direction specified on the parking lane?

I suspect direction=both will also exacerbate issues we have with Parking AI (something I know @krzychu124 wants to investigate once FPS Booster mod is done). It might be partially responsible for the 'floating to parking space' issue we already see.

But, on the other hand, we can't get all the asset authors to correct their roads in the workshop.

Maybe we have single pass of networks when a city loads and try and detect issues like this and correct them where possible? Usually parking is at outer edge of road, and thus easy to determine which way it should face either by looking at adjacent lane or just determining what direction traffic would flow on that side of road. But there are some unusual roads with parking in the middle, eg. between two medians, or adjacent to middle, eg. next to a central median, that might prove more difficult to correct.

The speed limits manager already has to scan all networks (#510, #513) to determine which are viable for speed limits (as we were getting all sorts of weird things showing speed limits signs in default speed limits UI, eg. decorative networks). Maybe we split that out in to a central network validator that is used to supply a ready-made, and auto-corrected where possible, list of network prefabs to all tools? We could also, as part of that, create bitmasks that describe what lanes are available (eg. https://github.com/CitiesSkylinesMods/TMPE/issues/516#issuecomment-586623805).

kianzarrin commented 4 years ago

@aubergine10 I don't think direction will matter when I use laneIndex for the backend. that is why I mixed it with this issue.

The speed limits manager already has to scan all networks (#510, #513) to determine which are viable for speed limits (as we were getting all sorts of weird things showing speed limits signs in default speed limits UI, eg. decorative networks). Maybe we split that out in to a central network validator that is used to supply a ready-made, and auto-corrected where possible, list of network prefabs to all tools? We could also, as part of that, create bitmasks that describe what lanes are available (eg. #516 (comment)).

I think maybe we can cache NetInfo and calculate values like (for example we can have a array of NetInfoExtras NetInfoCache[prefabColection<NetInfo>.CountLoaded()-1] )

originalfoo commented 4 years ago

central position for each direction: then during run time...

distance of each car lane to each of the parking lanes

Why would we do stuff at runtime? And why store distance?

When a city loads, or as it's loading (but only after mods like NExt2 and CSUR have added their roads), we could just scan all the network prefabs:

Then if we know what lane a vehicle is driving on, we can get the id of it's best choice parking lane.

But if we're going to do that, why not plan to create our own representation of a netinfo (etc)? Like, there's loads of info in a prefab that TM:PE, pathfinder and vehicle AIs just don't care about. We could create a compact descriptor with data structure that's optimal for what TMPE, AIs and pathfinder need. Each time we find an expensive calculation that could be done in advance and cached, we can update our descriptor to contain the computed info = start reducing needless per-frame and/or per-vehicle calculations = faster framerate.

kianzarrin commented 4 years ago

@aubergine10

distance of each car lane ... https://github.com/CitiesSkylinesMods/TMPE/issues/516#issuecomment-586526219 -- follow the link and you see this comment -->:

// randomize target position to allow for opposite road-side parking

You have asked several questions about the code in the link I think you should have a look at the code.

EDIT:

create our own representation of a netinfo (etc)

Yes that exactly what I meant. calling it cache was poor wording on by behalf.

originalfoo commented 4 years ago

Is randomization the correct approach? I think for small roads where m_canCrossLanes = true it would be viable if the car can u-turn mid-segment to go in to parking space on opposite side.

But for most roads, particulalry anything with more than 2 lanes of oncoming traffic, it seems odd to encourage cars to go wandering - perhaps for quite some distance - for a turning point and then drive all the way back just to reach parking spot on the other side of road.

kianzarrin commented 4 years ago

But for most roads, particulalry anything with more than 2 lanes of oncoming traffic, it seems odd to encourage cars to go wandering - perhaps for quite some distance - for a turning point and then drive all the way back just to reach parking spot on the other side of road.

the randomizer is not even based on the position of the car. its based on segment center +random displacement vector. If I am not mistaken its better to just choose a random parking lane.

do we even consider the length path the car has to travel to reach to the parking? because to me it seems to be purely based on distance of the car position to center position of the segment with parking.

it is not necessarily the same segment the car is parked in. the code I mentioned is in an spiral loop:

            LoopUtil.SpiralLoop(centerI, centerJ, radius, radius, LoopHandler);

By that time the road side does not matter anymore I guess.

kianzarrin commented 4 years ago

I am so going to create our own representation of a netinfo to solve this issue. its going to save both memory and speed :)

originalfoo commented 4 years ago

I am so going to create our own representation of a netinfo to solve this issue. its going to save both memory and speed :)

It's something this mod has needed for long time. By doing some processing up front we can probably drastically reduce per-frame processing in many places. Really looking forward to seeing what you come up with.

kianzarrin commented 4 years ago

It's this mod has needed for long time. By doing some processing up front we can probably drastically reduce per-frame processing in many places. Really looking forward to seeing what you come up with.

the speed limit tool can modify default speed limits. it has its own representation of a netinfo. I just need to extend it.

Alternatively each manager can have its own representation of netinfo. it does have the advantage of making each manager self sufficient but it makes information sharing a bit harder.

originalfoo commented 4 years ago

I'd prefer a "single source of truth" net info otherwise we end up having to constantly grab from different lists rather than just one list.

In my mind it would work something like this:

Also, thinking this forward, if we start allowing lane customisation (eg. the ability to turn bike lane in to parking lane or turning lane...) we'd might need a OurNetInfo BasedOn member which could link back to the original. So if we want to reset to default, we just change the netinfo associated with a segment back to the one it's based on.

You could then take that even further that we could start adding or removing lanes. :) And if you take that even further, given some 'chunks of mesh and textures' we could actually generate our own road meshes/textures based on our special netinfos. Instead of subscribing loads of roads, users could create their own road prefabs in-game, like "I'll have a road this wide with pavement either side and a median and three lanes each way with tram tracks on the innermost lane and paint it in style of US road markings and maybe some shrubs to make it look nice". It would be like Network Skins on steroids; instead of just making minor changes, you could build completely custom roads and even bundle them in the savegame file to make it shareable.

originalfoo commented 4 years ago

No idea what we'd call "our net info" so just referring to it as OurNetInfo for now. It would probably be better named something like AbstractNetInfo?

NetInfo <---> OurNetInfo  <---> OurNetInfo.BasedOn(the other one)
                  ^                           ^
                  |                           |
               some segment             another segment
kianzarrin commented 4 years ago

@aubergine10 whats that ? a linked list? I think an array based on PrephabColection<NetInfo>.GetLoadedCount() would do.

We have ExtSegmentEnd, ExtSegment so why not ExtNetInfo? it extends netInfo after all.

in speed limit tool manager its:

        internal Dictionary<string, float> CustomLaneSpeedLimitByNetInfoName;
originalfoo commented 4 years ago

Yes, ExtNetInfo is fine.

I assume each segment on the map has it's own copy of the netinfo? So it can be customised on segment by segment basis (in much same way network skins can change trees for individual segment)?

kianzarrin commented 4 years ago

@aubergine10 No. each road type has only one copy of NetInfo that cannot be copied (easily). The PrephabColection<NetInfo> holds a collection of the prefabs which can be accessed by an index. Each segment has a index to get the NetInfo from PrephabColection<NetInfo>.GetLoaded(index)

The way NS2 works is that for every new skin, it creates a new Skin instance which can be shared by multiple segments. There is an array of segmentId->Skin. each skin is based on a NetInfo. multiple Skins can be based on the same NetInfo.

In TMPE we do not want to do that. we want one ExtNetInfo instance per NetInfo instance. With the exception of SpeedLimitManager, ExtNetInfo does not change NetInfo but rather caches useful information about it for fast future access.