On a side note, I still need to do some testing to make sure all plugins are displaying the proper output.

However, GetWorkshopMapBaseName has been tested on Linux TF2 and CS:GO servers by itself and it seems to be working.

These plugins were updated to switch calls to the GetFriendlyMapName stock to the GetWorkshopMapBaseName native and a variable name change here and there.

Having said that, RockTheVote's change is new and hasn't been tested at all.

I did some testing on it and things seem to be working as expected.

I also merged the current master into it.

I did notice some minor naming issues I'm going to clean up, though (like *map instead of *pMapName on the C++ .h file)

The basics of this look okay to me, but I only skimmed through the mapchooser+friends edits.

If it's all tested, we should be good. My biggest concern there would be to make sure that we're still storing the full map "name" for votes, nominations, etc. rather than the new base name, as a base could potentially be ambiguous and should only be used for display.

@psychonic As you said, the base name is for display only. Generally, we create a variable for it, retrieve it, display it (or add it to a menu in the display name field), then let it go out of scope.

Having said that, the name returned from FindMap (not GetWorkshopMapBaseName) may be stored in adt_arrays. Luckily, FindMap is safe to call on map names returned from FindMap, so that should be a non-issue.

Can we go for just GetMapBaseName (or GetMapDisplayName) instead?

@asherkin I'm not particularly attached to the function name, so we can change it.

Should we just go with GetMapDisplayName then? It probably describes the function's purpose better than GetMapBaseName.

GetMapDisplayName sounds good to me, @psychonic?

I'm going to push this for now so I don't forget. If @psychonic doesn't like the name, we can always change it again later.

@asherkin @psychonic One question.

I've noticed that in some plugins I've updated LogAction to display the display name and some I didn't.

Since this behavior should be the same across plugins, should we be displaying the display name or the original map name in LogAction?

I ask because I'm not sure which we should use due to OnLogAction's existence.

GetMapDisplayName sounds good to me, @psychonic?

Yeah, that's fine.

@powerlord I think that logs should give the full map name or path, which display to users such as in chat or menus should use the shortened display name. @asherkin thoughts?

Hmm, should I update the documentation for GetMapDisplayName to note that it will return the result of FindMap for non-workshop maps?

Edit: Derp, of course I should.

@psychonic @asherkin Right now, GetMapDisplayName returns true if FindMap returns FindMap_Found, FindMap_FuzzyMatch, or FindMap_NonCanonical. Should I update it to only return true if the displayName is different than the map name passed in?

Yeah, I heard of an issue with snprintf on Windows, so I removed some of what I think is redundant logic (now that we overwrite the . with \0) that should lessen the chance of it happening.

Needs testing, of course.

Still, make sure you pass PLATFORM_MAX_PATH for the nMapNameMax variable and it won't ever happen.

The previous change has been tested as working on Linux, don't have a Windows server to test it, but it should work.

test_mapdisplayname workshop/454141042
GetMapDisplayName says "cp_kakariko_b1" for "workshop/454141042"

I ran into a new issue, though.

If you sync a lot of maps in server.cfg, the game server may not have them all cached by the time it generates the nominate list or voting list during the first map, which is done in OnConfigsExecuted.

Should we delay creating these menus/votes until the first time we use them in a map?

The previous changes dealing with late loading the nominations list and list of maps for the next vote appear to be working.

To clarify, the map list is not being late loaded, only the arrays and menus that use it are, in order to do late name resolution on them.

I'm kinda glad this hadn't been merged yet, as I just spotted a bug in the CS:GO code for GetMapDisplayName... when I fixed the pointer math to array syntax in commit 14d62b0, I used the wrong array.

Sorry that I haven't had a chance to finish reviewing it. It's been a busy past few weeks,

I just force reset back a commit. Turns out the sourcepawn commit referenced in sourcemod-git5533 is very buggy.

Edit: Ugh, the previous master merge has the same exact issue.

Ended up rolling back the previous pull from master and redoing the GetMapDisplayName regression fix.

I'm not sure why, but TF2's FindMap seems to no longer resolve workshop map names like it used to.

For example, immediately after seeing this in the log (because I did tf_workshop_map_sync):

[TF Workshop] Got updated information for map [ workshop/cp_glassworks_rc6a.ugc454118349 ]

this happens

test_findmap workshop/454118349
FindMap says PossiblyAvailab - "workshop/454118349"

It used to return NonCanonical and copy the longer map name into the map variable.

Short names are still resolved, such as cp_dust to cp_dustbowl.

I emailed Valve's John Schoenick about this issue. His response:

FindMap does not return Found unless it has the map fully downloaded and available -- the message in question just means we have full information about the map, not that it is installed. I'll tweak it in the next update so that it still provides the full name, if known, in that situation, but otherwise this is working as intended.

I have replied to John pointing out the following:

1. The test I performed was 10 hours after the tf_workshop_map_sync command was initially issued, although it was issued again on map change.
2. The map in question is the same map I've had on my server since I initially started testing FindMap a week or so after Workshop support was added. It was updated once since then.
3. FindMap used to return eFindMap_NonCanonical and update the map name for maps that were already on the server.

He hasn't responded to that yet, though.

The above behavior I'm seeing is a bug:

Can you switch to this level with changelevel ? There is a bug in the currently release build that could cause UGC files to never finish downloading. We hope to have a fix for that soon, but it would be good to rule that out.

After doing more testing, I can't switch to workshop maps, even ones that are completely downloaded... it just tries to re-download them and the server soft-hangs (i.e. the console still works, but no commands are processed). When I sent John my logs, he said:

That sounds like the aforementioned bug, which will be fixed by the next TF update. That would explain why FindMap() never considered it available as well :-/

Looks like today's TF2 update should fix the issues mentioned in the previous posts. Needs testing still, though.

Did some testing after today's update.

All Map Workshop functionality appears to be working as intended.

The engine at the moment in FindMap will write out 100 characters to an internal buffer then do the scan.

If you pass a length of 0, you can pass your const's in there and they should come out clean. This should avoid having to copy your buffer over and over. I'm really mixed on breaking this API in pawn and explicitly getting you guys to pass a const in instead of the following

char dispMap[24];
strncopy(dispMap, sizeof(dispMap), map);
new SMMapResult res = FindMap(dispMap, sizeof(dispMap);

I find this pattern to be really harmful in pawn, we have nothing else like it and it seems really bogus. In core at the moment we have IsMapValid creating a bogus temporary buffer to use the valve api.

@KyleSanderson Last time I checked, attempting to pass a const char[] to a method that takes a char[] will get you a compiler error in both SourcePawn and C++.

#396

Current build won't compile the plugins, but after fighting git for the last 20 minutes before finally convincing it that the submodule versions really did change (it kept claiming they were up to date on the old versions), I'm going to do a "safety save."

@powerlord you now have 21 commits in this branch and it's two months old, you should most likely rebase it once you have everything working.

@KyleSanderson Question about that... since I've merged master into this several times, do I just select all the commits and combine them? I ask because all the commits made to master also show up in the commit log for this branch.

@KyleSanderson

There, I think I rebased this correctly. I had to watch it as it told me every time I changed HalfLife2.cpp that there was a conflict, but there was one instance where my change was the correct one (It was the one where the command name changed from GetWorkshopMapBaseName to GetMapDisplayName).

It compiled using AMBuild on Windows at any rate.

I've amended the commit message as it was a mess after the rebase earlier.

Let's get this into the dev branch. We can iterate on it from there if necessary.