Skip to content

Fix Network Refresh overwriting properties marked with FromHost SyncFlag#77

Closed
andy013 wants to merge 131 commits intoFacepunch:masterfrom
andy013:fromhost_fix
Closed

Fix Network Refresh overwriting properties marked with FromHost SyncFlag#77
andy013 wants to merge 131 commits intoFacepunch:masterfrom
andy013:fromhost_fix

Conversation

@andy013
Copy link
Copy Markdown
Contributor

@andy013 andy013 commented Dec 1, 2025

Issue: #564

Changes

  • Skip [Sync] properties in PostDeserialize during a network refresh.
  • Check if connection has control when reading data table after a refresh

Notes

This fix prevents clients who call a network refresh from overwriting Sync properties that have the FromHost SyncFlag.

Currently, [Sync] properties are deserialised from the client's JSON payload, overwriting authoritative host values. ReadDataTable also lacked a check, causing NetList and NetDictionary types to be reset.

Checks are performed on the receiver side. I figured you should always have the receiver check if the sender was allowed to change the value to prevent hacked clients sending data and it just being blindly accepted. You could probably optimise it by also not sending FromHost values on a refresh called by clients.

I've not tested this extensively but it seems to be working in my basic tests. You should review it and make sure I'm not doing anything stupid.

Example

Test project: fromhostbug.zip

Join with a client and observe server console logs. Client calls Network.Refresh every frame which overwrites the values set by the host.

OnStart() sets the Sync FromHost properties like this:

protected override void OnStart()
{
	if ( Networking.IsHost )
	{
		Ammo = 6;
		NetworkList.Add( 1 );
		NetworkList.Add( 2 );
	}
}

Before fix:

[Generic] Wilson [76561197988413004] is connecting	
[Generic] [HOST] Ammo: 0 -> 1	    <--  Ammo set to 1 from the prefab
[Generic] Wilson (2) [76561197988413004] is connected	
[Generic] [HOST] Ammo: 1 -> 6	    <--  OnStart called for host
[Generic] List Changed: 	
[Generic] 1	
[Generic] 2	
[Generic] [HOST] Ammo: 6 -> 1	    <--  Client refresh sets ammo back to prefab value!
[Generic] List Changed: 	
[Generic] List is now empty.        <--  NetList also reset!

After fix:

[Generic] Wilson [76561197988413004] is connecting	
[Generic] [HOST] Ammo: 0 -> 1	
[Generic] Wilson (2) [76561197988413004] is connected	
[Generic] [HOST] Ammo: 1 -> 6	
[Generic] List Changed: 	
[Generic] 1	
[Generic] 2	     <-- Both Ammo and NetList stay set to host values even after client calls Network.Refresh

sboxbot and others added 30 commits November 24, 2025 09:05
This commit imports the C# engine code and game files, excluding C++ source code.

[Source-Commit: ceb3d758046e50faa6258bc3b658a30c97743268]
…cene.PhysicsWorld (Facepunch#3433)

Resolves Facepunch/sbox-issues#9598 (Fixes issues where traces in the editor can throw NREs after exiting play mode)
* Skip menu editor code outside of the menu project
* Asset Browser: skip menu project assets in Everything and Recents
Add slight delay because windows may still briefly hold a lock on some files even after the process exited
* Refactor sync script

Use single filter repo call
Use globs instead of python madness
Ensure shallow clone is clean before working on it

* Add dry run option to sync script
* Add public gitattributes
* Fix leaky swapchain when resizing on game mode and dispose of handle after usage
VideoRecorder would get a strong handle copy of the swapchain every frame and retain it, causing native to fail to shutdown the copies from it
Made usage of getting native swapchain consistent on managed on other stuff, doing it like this ensures GC properly disposes of the strong handles even not disposing explicitly

Remove now unused ScreenRecorder.def and ScreenshotService.def

There are still optimizations to swapchain I'd like to send on another commit, game is allocating way more swapchains than needed even without the leak, has two completely different paths depending if you are MSAA or not, this can all be much simpler
https://files.facepunch.com/sampavlovic/1b1811b1/EjDyxbTahs.png

* Remove NativeLayerRenderTarget, was unused and fucked

* Keep it as an ITexture/HRenderTextureStrong in managed so we avoid IDisposable, ReadTextureAsync with ITexture
…h#3449)

* Support instanced tint on Blendable and Material::From( i )

Blendable supports vertex tint color, but was initially intended for world geometry so there was no point in working with instance color, now works fine

https://files.facepunch.com/sampavlovic/1b2611b1/sbox-dev_qkWlvBOXW8.png

* Build shaders
* More robust downloading of artifacts

Don't download to a temp file first
Try to retry download up to 3 times, if it fails
Fail Bootstrap if any download fails

* Fix contentbuilder and shadercompiler not forwarding to stdout
…eleted

Move filter implementation to python so we have more flexibility when implementing filters.
Make sure deleted files are properly filtered by evaluating globs in the filter itself not before.
Make sure deleted LFS files are accounted for by scanning the history of the shallow clone for delete/changed lfs files.
Whitelist some additional shaders.
* Fix terrain seams and optimize

Overlap LODs by one step to fix holes in LOD transitions
Reuse vertices that exist on same key when building diamond square

https://files.facepunch.com/sampavlovic/1b2411b1/8mb.video-eW2-tNb22a60.mp4

* Add NoTile class and make terrain use it
https://files.facepunch.com/sampavlovic/1b2411b1/sbox-dev_R1FwUmLhvu.mp4
https://files.facepunch.com/sampavlovic/1b2411b1/sbox-dev_YhKyIwvhve.mp4

* Sure why not Mr. Robot
* SceneEditorSession: make game vs editor scenes more distinct, Scene is whichever is currently active

* Route prefab update, model reload etc events to both scenes if needed

* SceneTree update checking a bit cleaner

* Bring back GameEditorSessions instead, so undo, selection etc can all be linked 1:1 with scenes again

* tweak and tidy
handsomematt and others added 19 commits December 2, 2025 17:41
* Add r_max_anisotropy setting

Our anisotropy would default to the maximum value supported by hardware (16x), this trashes perf on my Intel B580, unreal does not even support above 8x

Unlike r_texturefilterinquality, it does not clog the renderer with user config samplers, r_max_anisotropy 1 has same result as trilinear rendering

Set default to 4, seems a good value

Not hooked up to UI

* Add r_max_anisotropy to quality_profiles
…idget (Facepunch#3540)

* Fix vsnd playback skipping the first few milliseconds

only increment Time after everything else has finished happening, this fixes the first few milliseconds of audio being cut off

Ensure SamplesPerColumn is greater than 1, this fixes the waveaform being a solid line if a sound file was too short and the inspector was too wide

Fix subsequent playback skipping time by setting time to 0 after EditorUtility.PlaySound

Use SmoothDelta to fix inconsistent playback

Co-authored-by: DrakeFruit <foxflowgaming@gmail.com>
- Fix navmesh potentially generating twice
- Make sure it's generated async on load.
- Only generate it in editor scenes if actually used
* Fix default %project%/Assets/ location when saving prefabs + scenes
* Display error message box if it's not a valid assets location
* Tweak "File > Save" enabled conditions to match if they'll do something
@andy013 andy013 marked this pull request as ready for review December 3, 2025 17:00
solwllms and others added 6 commits December 3, 2025 17:27
They are essentially compile time constants, so we shouldn't allocate them
* Make MSAA setting work on editor SceneRenderingWidgets

MSAA settings never worked with editor viewport, only on game tab, now with playmode it always takes highest MSAA value from hardware regardless of what

Two things from this PR:
* Pass current engine MSAA setting when creating SceneRenderingWidget swapchain
* Add callback when video settings are changed, recreate swapchain when so

Replicates fine for all SceneRenderingWidgets, applies instantly

https://files.facepunch.com/sampavlovic/1b0311b1/sbox-dev_bjEMrrYe71.mp4

* Push RenderMultisampleType_t nMSAAAmount on interop
if ( options.IsNetworkRefresh && !options.IsRefreshFromHost )
{
// Skip [Sync] vars - sent via network table (prevents FromHost overwrite)
if ( field is PropertyDescription prop && prop.HasAttribute<SyncAttribute>() )
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I use ReflectionQueryCache.SyncProperties instead of prop.HasAttribute here?

/// If true, the network refresh originated from the Host,
/// so we should allow it to overwrite [Sync] properties.
/// </summary>
internal bool IsRefreshFromHost { get; set; }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We hould really avoid adding new flags to options whenever possible. Is there no other way?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.