Add functions for working with entity lumps#1673
Add functions for working with entity lumps#1673Headline merged 12 commits intoalliedmodders:masterfrom
Conversation
|
Have tested and confirmed that it's working now. This plugin is available as a sample for entity lump access; it's a copy of the extension's version with the replacement of |
There was a problem hiding this comment.
This looks solid, I like the API a lot - thanks for doing the work here @nosoop! The usage does seem niche in total plugin count, but I think it's powerful enough (and stripper popular enough) that it does make sense to ship with SM for sure.
The implementation looks good, I haven't super dug into it yet but I know it's been well tested. The interaction between weakref and Handle could be a bit interesting, but I think it's definitely an easier way to go than dealing with handle cloning (and handles aren't the sanest in native land currently). @Headline I'd appreciate your eyes on the C++ stdlib usage here as an extra sanity check.
I think the only change to the implementation that I think it'd be good to attempt is moving the bulk of the code into the logic binary (LumpManager and the natives) - the per-engine builds really are slow and we're trying to share as much as possible, and it looks like we won't need much added to the bridge to accommodate this.
EDIT: Please could you include that test plugin in plugins/testsuite/ too - it looks like a really great reference.
This reallows for building the standalone parser for testing.
This allows for standalone testing of the parsing component with arbitrary entity strings.
|
As long as there isn't any need for SDK-specific flags, moving it into the logic binary would be fine. I'm not familiar with how the bridge itself would work. From skimming the source, it sounds like I should move the files into Aside, I'm also doing some minor refactors to move the SourceMod dependencies out of
Will do; I'll push a commit with that. Before I forget to mention it, there are currently no checks in place to ensure users don't insert double quotes [in keys / values]. I've considered it before, but I'm not sure which of the options would be preferable:
|
|
I've moved the entity lump code into the logic binary and confirmed that it builds and runs as expected on Windows / Linux TF2, and have also added the standalone entity lump parsing utility under I'd also request to please add |
Co-authored-by: Mat <[email protected]>
|
Any news regarding this pull request? |
Nothing on my end; just waiting on the merge or further feedback. I might retract the PR since it missed 1.11 to fill in for the now-removed I can't think of a good migration path from the extension to the in-core version, which is why I didn't want to publish extension builds myself.
The lumps remain available in case plugins have any reason to read the original data in the future. Should be able to pool the keys, though. |
|
Retracting this PR; extension builds will be made available within the next few days*. I'm open to resubmitting this in a form that is backward-compatible with users of the extension in the future. * unless a maintainer merges this into 1.11+ or gives me something actionable before builds go up, I guess — the important thing is that users on current stable builds are able to work with this |
|
Reopening this and moving the informal discussion with @Headline from Discord into the PR conversation here. Hopefully we can get this sorted out and into stable soon.
If I remember correctly, the idea was that Still, a plugin shouldn't remove an entry it's working with, and it shouldn't expect anything to be preserved if it calls into another plugin, so I consider it developer error. I can expose a property to check for expiry, but in my mind, plugins should not have to worry about it; granted I'm sure people will find creative ways to break my assumptions.
To clarify, the permissions currently in the PR (in Last thing I was considering was tweaking the method naming to match those of |
If the entity lump string cannot be fully parsed, the manager will have a list of successfully parsed lump entries up to the point of failure. This commit prevents the serialized partial entity lump from replacing the original entity string in SourceModBase::GetMapEntitiesString. However, the lump entries will still be present for plugins to work with; this is not defined behavior and we may opt to empty out the lumpmanager instead. This regression was introduced during the migration from extension to core; the extension version never calls its OnMapEntitiesParsed forward on parse failure, so plugins were previously blissfully unaware of entities that may be manipulated, though they could still perform read-only access on the partial list past that forward.
|
Fixed up a bug introduced in the core-merged version that would cause issues if parsing failed partway into a file. For those interested in testing, attached are builds of Windows and Linux versions compiled for supported SDK versions. The builds were compiled against entlump-1.11, which is this PR rebased against a somewhat recent 1.11 commit (5d9ddee causes rebase conflicts, presumably due to a file list being reverted to a previous ordering). package.zip (Windows) / package.tar.gz (Linux) I've tested the build on TF2, so I assume there won't be any regressions for the few others that have run the extension; the lumpmanager component is practically unchanged. If an entity lump parse failure occurs, it should report an error, but otherwise the server should continue to function as before. |
|
LGTM! Minor nitpick: Would it make more sense to turn |
|
Thanks for testing!
|
* Fix support for SDKCall returning non-networked entity (alliedmodders#1797) (cherry picked from commit 625c7a9) * Update SourcePawn on 1.11-dev. * Revert "Fix use-after-free when creating custom user messages" This reverts commit 15450a6. * Revert "Introduce a pbproxy library to solve macOS linker issues." This reverts commit e5ddbd9. * Fix build. * Update SourcePawn. * Update gamedata for 2022/10/21 CS:GO update (alliedmodders#1842) * Trigger build for hl2sdk-csgo update * Correct missed team offsets in CheckRestartRound (alliedmodders#1844) * Trigger build for hl2sdk-csgo update * Fix DHooks jit code stack memory alignment (alliedmodders#1849) * Enable CI on release branches (alliedmodders#1854) * Add functions for working with entity lumps (alliedmodders#1673) * entitylump: Fix behavior of append (alliedmodders#1836) This change ensures that the iterator values used by `std::distance` is correct. Having the emplace within leads to the possibility of `m_Entities.begin()` being invalidated due to reallocations. * Update TF2 gamedata. (cherry picked from commit 89bd4d7) * Ensure gameconfig file uniqueness when reading master.games (alliedmodders#1859) The extended gameconfig format reads the master gameconf file twice; once each for the base engine and actual engine. The file list isn't checked for duplicates, so 'common.games.txt' is loaded in twice, resulting in any 'common' file values potentially overriding values listed under '#default' in other files due to bShouldBeReadingDefault. This happens in the case when matching game versions by CRC (such as public game branches). Required for alliedmodders#1857. (cherry picked from commit 34c8220) * Trigger build against SDK update * Trigger build for TF2 SDK update * entitylump: Output separator as spaces instead of tabs On NMRiH and possibly other games that use the Maphacks system, entries that are modified using that system are rendered with tab characters stripped out - see CNMRiHMapHackManager::GetEntDataString. That results in there being no separators at all between keys and values, as Maphacks receives the serialized string from Entity Lump Manager. This commit changes the key / value separator character to use spaces instead. This discovery upsets me greatly. Fixes alliedmodders#1833. * Add missing null pointer check to protobuf messages (alliedmodders#1883) (cherry picked from commit ef7d3ab) * Fix ReadMapList ignoring file's last modified time (alliedmodders#1891) (cherry picked from commit 0d61792) * Update TF2 gamedata for version 7757534 (2023-01-05) (alliedmodders#1901) (cherry picked from commit 27b1817) * Gamedata update after 2/2/2023 CSGO update (alliedmodders#1921) * Linux [SDKTOOLS] Sigscan for FireOutput FIX (alliedmodders#1923) Addie said: test the signatures: \x55\x89\xE5\x57\x56\x53\x81\xEC\x8C\x01\x00\x00\x8B\x55\x08 or digby's \x55\x89\xE5\x57\x56\x53\x81\xEC\x8C\x01\x00\x00\x8B\x55\x2A\x65\xA1 \x55\x89\xE5\x57\x56\x53\x81\xEC\x8C\x01\x00\x00\x8B\x55\x08 fixed the issue for linux server. (cherry picked from commit 2dcee81) * Fix EntityFactoryCaller signature (alliedmodders#1925) (cherry picked from commit 6e839a9) * Fix LookupAttachment signature (alliedmodders#1933) (cherry picked from commit a0eb6a9) * Enable ShowMenu support for Reactive Drop (see alliedmodders#1938) Note: Currently only works on beta version of game (cherry picked from commit 397b70a) * Fix LookupAttachment signature for NMRiH (alliedmodders#1940) (cherry picked from commit 693e584) * Add LookupAttachment signature for ND (alliedmodders#1942) Adds the LookupAttachment signature for Nuclear Dawn (cherry picked from commit a9a1939) * Added hack to make plugins open a menu with all possible targets on ReplyToTargetError COMMAND_TARGET_AMBIGUOUS. Explanation: There are two clients in the server, one named gene, the other one "Ene ~special characters~". An admin issues "sm_slay Ene" and gets following error message: More than one client matched the given pattern. What this hack will do is: Use GetCmdArg(0, ...); to get the command name "sm_slay". Use GetCmdArgString(...); to get the arguments supplied to the command. Use GetLastProcessTargetString(...); (which was implemented in this commit) to retrieve the arguments that were passed to the last ProcessTargetString call. It will then pass this data to the DynamicTargeting plugin through its AmbiguousMenu native. The plugin will open up a menu on the client and list all targets which match the pattern that was supplied to ProcessTargetString. If the client selects a menu entry, FakeClientCommand will be used to re-execute the command with the correct target. * feat(ci): release tar gz --------- Co-authored-by: 42 <[email protected]> Co-authored-by: David Anderson <[email protected]> Co-authored-by: Vauff <[email protected]> Co-authored-by: Nick Hastings <[email protected]> Co-authored-by: Maxim Telezhenko <[email protected]> Co-authored-by: Erin Baker <[email protected]> Co-authored-by: nosoop <[email protected]> Co-authored-by: zer0.k <[email protected]> Co-authored-by: clague <[email protected]> Co-authored-by: GAMMACASE <[email protected]> Co-authored-by: El Diablo <[email protected]> Co-authored-by: Poggu <[email protected]> Co-authored-by: Corey D <[email protected]> Co-authored-by: Dysphie <[email protected]> Co-authored-by: data-bomb <[email protected]> Co-authored-by: BotoX <[email protected]> Co-authored-by: maxime1907 <[email protected]>
5 participants
Closes #1547.
This PR merges my Entity Lump Manager extension into SourceMod's core mostly as-is, with the primary change being that the lump is unlocked for writing during
OnMapInitinstead of during its own forward.Making use of
OnMapInitjust about the only reason for merging into core — I'm open to alternative options if the team would like to push forward with its inclusion in some form.Parsing logic tested against ~700 maps for Team Fortress 2, and a bug has been reported and fixed for CS:GO.
Also open to other changes like bringing the native names more in line with the existing
ArrayListmethods and such.(PR is a WIP and not in working order — have to do some additional work for creating the handle type and to clean up the includes / license headers. Just putting the PR in now in case anyone has any input regarding its inclusion.)