Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
originalfoo opened this issue Sep 1, 2019 · 30 comments
Open

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

originalfoo opened this issue Sep 1, 2019 · 30 comments
Labels
enhancement Improve existing feature PARKING Feature: Parking AI / restrictions / etc UI User interface updates

Comments

@originalfoo
Copy link
Member

originalfoo commented Sep 1, 2019

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 #516 (comment) this has to be removed after fix : https://github.com/CitiesSkylinesMods/TMPE/wiki/Notes-for-Asset-Creators#parking-lanes

@originalfoo originalfoo added enhancement Improve existing feature UI User interface updates PARKING Feature: Parking AI / restrictions / etc labels Sep 1, 2019
@kianzarrin kianzarrin self-assigned this Feb 14, 2020
@kianzarrin
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Member Author

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
Copy link
Collaborator

kianzarrin commented Feb 15, 2020

@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
Copy link
Member Author

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
Copy link
Collaborator

kianzarrin commented Feb 15, 2020

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
Copy link
Member Author

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
Copy link
Member

krzychu124 commented Feb 15, 2020

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

@kianzarrin kianzarrin removed their assignment Feb 15, 2020
@kianzarrin
Copy link
Collaborator

@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
Copy link
Member Author

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
Copy link
Collaborator

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
Copy link
Member Author

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
Copy link
Collaborator

@aubergine10 lol! It works. lane index is available to path/parking finder as you can see in the code i provided in #516 (comment). 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
Copy link
Collaborator

kianzarrin commented Feb 18, 2020

Screenshot (554)

some UL roads are acting weird see #708 (comment) and #708 (comment)

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
Copy link
Member Author

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
Copy link
Collaborator

kianzarrin commented Feb 18, 2020

@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
Copy link
Member Author

originalfoo commented Feb 18, 2020

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. #516 (comment)).

@kianzarrin
Copy link
Collaborator

@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] )

  • central position for each direction: then during run time add that to segment.m_centerPosition to get the final m_centerPosition. (this is for UI and is not so performance critical unless overlay is turned on.)
  • distance of each car lane to each of the parking lanes. this means in the advanced parking manager we no longer need to call super expensive GetClosestLanePostion() function every simulation step for every person who wants to park! see Allow separate control of parking lanes on 1-way streets. #516 (comment)
  • I don't exactly understand the problem with the speed limit but it sounds like caching them can help. at least we can store a boolean to see if we should go through fast code path or slow code path.

@originalfoo
Copy link
Member Author

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:

  • Work out if it's something TMPE should be interested in -> if so, add to a dictionary(?) keyed by vanilla object id
    • The dictionary (or whatever type of reference list is fastest) would get cleared when level unloads or a hotswap occurs, etc.
  • If road, then for each vehicle lane find closest parking lane going in same direction -> store parking lane id as property of the vehicle lane

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
Copy link
Collaborator

kianzarrin commented Feb 19, 2020

@aubergine10

distance of each car lane ... #516 (comment) -- 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
Copy link
Member Author

originalfoo commented Feb 19, 2020

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Member Author

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
Copy link
Collaborator

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
Copy link
Member Author

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:

  • Go through list of all netinfos
  • Filter out anything we're not interested in (eg. those misconfigured assets - like these - that look like they support vehicles but actually don't)
  • In the stuff that remains, fix up any errors (eg. direction = both on parking lanes)
  • Compile everything in to our special optimised netinfo
  • Make tmpe use only our optimised netinfo

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
Copy link
Member Author

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
Copy link
Collaborator

kianzarrin commented Feb 19, 2020

@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
Copy link
Member Author

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
Copy link
Collaborator

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve existing feature PARKING Feature: Parking AI / restrictions / etc UI User interface updates
Projects
None yet
Development

No branches or pull requests

3 participants