Commit graph

14 commits

Author SHA1 Message Date
Kamron Batman
f4a771c19d
fix: Items dropped on the ground never decay (#2536)
## Problem

Items dropped on the ground never decay. Corpses do, which makes the breakage look selective — but corpses are unaffected only because `Corpse.BeginDecay` runs its own `InternalTimer` and never touches `DecayScheduler`. Ordinary items are the only things that depend on the scheduler.

## Root cause

`Item.MoveToWorld` called `SetLastMoved()` — which triggers `UpdateDecayRegistration()` — at the *top* of the method, before detaching the item from its parent and before assigning the new map. `CanDecay()` reads `Decays`, `Parent`, **and** `Map`, so registration was evaluated against the item's *pre-move* state.

Because the `Item` constructor sets `m_Map = Map.Internal`, and `Mobile.Lift` calls `item.Internalize()` to put an item on the cursor, registration was consistently one step behind:

| State | Tracked for decay? | |
|---|---|---|
| Item on the ground | **No** | never decays |
| Item held on the cursor | **Yes** | backwards |

Nothing corrected it afterwards: the later `m_Map = map` assigns the field directly, bypassing the `Map` property setter, and that setter does not refresh registration either. With no parent, `RemoveItem` (which *does* re-register) never runs.

World load masked this — `ItemPersistence.PostDeserialize` re-registers every item against its final state, so decay appears to work for items that survive a restart. Only freshly dropped items are affected.

**Fix:** stamp `LastMoved` up front so decay math stays correct, then call `UpdateDecayRegistration()` once the parent, map, and location are final.

## Audit of the rest of the call sites

All 16 `SetLastMoved()` call sites were reviewed. `SetLastMoved()` must keep refreshing registration — `LastMoved` feeds `ScheduledDecayTime` and therefore which bucket an item belongs in — but it may only run once parent/map are final. The vendor, house, lift and drop sites already satisfy that. The rest of this PR fixes the ones that did not, plus what the audit turned up:

- **`Item.Deserialize`** stamped via `SetLastMoved()` before the version data was read, registering against an unread `Map`/`Parent`. Safe only by accident (the `Item(Serial)` ctor leaves flags at 0, so `Decays` is false), and it cost an unregister per item per world load. Now stamps only; `PostDeserialize` does the registration.
- **`Item` constructor** registered then immediately unregistered every item — the `Movable` setter saw `m_Map` still null, so `CanDecay()` was true. Also removes a `Configure`-order landmine: constructing an `Item` before `DecayScheduler.Configure()` would have thrown in `Shared.Start()`.
- **`Container.Destroy`** stamped `LastMoved` immediately before `MoveToWorld`, which now stamps it itself.
- **`Unregister` was documented O(1)** but scanned twelve buckets and did a linear `PriorityQueue.Remove`, on every construction, deserialize and move. Items now record where they are tracked in `Item.DecaySlot` (1 byte), so untracked items — the common case — leave in O(1).
- **A refused decay silently dropped the item.** `ProcessActiveQueue` deleted on `OnDecay() == true` but did nothing when a region refused, leaving the item dequeued, untracked and on the ground forever. It now restarts the decay clock; re-registering as-is would spin, since `ScheduledDecayTime` is already past.

## Two content bugs of the same class

The decay system replaced a polling sweep. Under polling, a `Decays`/`DecayTime` override could read live state every pass. Under a registration model it cannot — the scheduler drops items that stop being eligible, but nothing enrols one that becomes eligible while untracked.

- **`TreasureChestLevel1-4`** overrode `DecayTime` as `Utility.Random(15, 60)` — a fresh roll on every read. `ScheduledDecayTime` is read repeatedly (to bucket, to re-bucket on rotation, to test whether due), so those reads disagreed: the chest re-bucketed every tick and decayed early instead of after its intended interval. Now rolled once per chest. Distribution unchanged — `Utility.Random(from, count)` is RunUO's `from + Next(count)`, so this is 15–74 minutes, as before.
- **`StrongBox`** overrode `Decays` with a live check on `_house`, `_owner.Deleted` and `IsCoOwner`. Nothing notifies the box when any of those change, so it was never enrolled and the override never decayed anything — and it could not have: `HouseRegion.OnDecay` refuses a secured item inside a standing house, and the box is in `Secures`. Decay was never the mechanism here.
  - A strongbox is only ever its owner's. Without a house, or without an owner still co-owning that house, it would be a free container anyone could loot, so `Validate()` destroys it. The old check missed exactly those two cases — it required a non-null owner and treated a null house as valid. A deleted owner deserializes back as null, which `IsCoOwner` rejects. The now meaningless `Decays`/`DecayTime` overrides and an unhelpful `Console.WriteLine` are gone.

`DecayScheduler` now documents both constraints.

## Tests

`DecayRegistrationTests` (Server) covers world placement, lift/drop, container round-trip, cursor-held (must *not* track), `Container.Destroy` spill, `DecaySlot`/structure agreement, refused decay, and a full decay lifecycle driven through the scheduler. `TreasureChestDecayTests` (UOContent) locks `DecayTime`/`ScheduledDecayTime` stability across all four chest levels.

To make the lifecycle testable deterministically, `DecayScheduler` gains `internal` members (visible only via existing `InternalsVisibleTo`): `IsRegistered()`, `ProcessTick(now)` — extracted from `OnTick()` with no behaviour change — and `ResetForTests()`.

Red/green verified. Without the `MoveToWorld` fix, 5 of 6 of the original tests fail, including `ItemOnGround_ActuallyDecaysAfterDecayTime`, which shows a ground item never decays even after a full simulated hour. Without the chest fix, 8 of 8 chest tests fail. Without the refused-decay fix, that test fails.

Server.Tests 737/737 and UOContent.Tests 509/509 pass.
2026-07-16 18:51:04 -07:00
Kamron Batman
4836bff5eb
fix: Eliminates List allocations in various places. (#2158) 2025-11-16 18:33:53 -08:00
Kamron Batman
da517f56c4
chore: Fixes variable types for possible performance issues (#2172) 2025-05-01 21:54:41 -07:00
Fabrizio
537526a328
fix: Update LabelTo, SendMessage, etc to Interpolated Strings (#1283) 2022-11-28 19:58:12 -08:00
Kamron Batman
936000ecf7
feat: Adds SerializedCommandProperty (#1249)
## Breaking Change
* Removes `SerializableFieldAttrAttribute` in favor of `SerializedPropertyAttr`.

## Important Change
* Adds `SerializedCommandProperty`

Example:
```cs
    [InvalidateProperties]
    [SerializableField(0)]
    [SerializedCommandProperty(AccessLevel.GameMaster)]
    private Mobile _completedBy;
```
2022-11-14 18:24:33 -08:00
Kamron Batman
ecbee17690
fix: Optimizes OPL using string interpolation (#1041)
## Breaking Changes (New API)
ObjectPropertyList supports the following API:
```cs
list.Add(500000);
list.Add(500001, stringArgument);
list.Add("Some text");
list.Add($"Some text with {argument}");
list.Add(500002, $"{arg1}\t{arg2}");
```

## Notes
1. All API uses that require a formatter like this:
    ```cs
    list.Add(500002, "{0}\t{1}", arg1, arg2);
    ```
    Should be changed to use string interpolation, for example:
    ```cs
    list.Add(500002, $"{arg1}\t{arg2}");
    ```
2. The following paradigm should no longer be used:
    ```cs
    list.Add(1061170, prop.ToString()); // strength requirement ~1_val~
    ```
    The new string interpolation API will avoid having to convert the argument to a string before writing it to the packet. Instead use the following:
    ```cs
    list.Add(1061170, $"{prop}"); // strength requirement ~1_val~
    ```

### Benchmarks
```cs
|                         Method |     Mean |   Error |  StdDev |  Gen 0 | Allocated |
|------------------------------- |---------:|--------:|--------:|-------:|----------:|
|                BenchmarkOldOPL | 241.0 ns | 0.56 ns | 0.47 ns | 0.0105 |      88 B |
| BenchmarkStringInterpolatedOPL | 199.9 ns | 2.44 ns | 2.39 ns |      - |         - |
```

### Changes
- [X] Removes crash in STArray.Return when array is null.
- [X] Fixes NPE in OPL when entity is null. Serial in packet will be 0 when entity is null.
- [X] Fixes NPE in AosAttributes when Parent is null.
- [X] Changes OPL to use string interpolation.
- [X] Introduces `IPropertyList` to allow extending PropertyList for other uses.
2022-06-02 10:09:53 -07:00
Kamron Batman
54d728a322
fix: Updates serialization to use v2.0 (#998)
- [X] Deletes serialization annotations
- [X] Updates to ModernUO.Serialization.Annotations nuget
- [X] Updates serializer to v2.0
- [X] Changes all `Serializable()` to `SerializationGenerator()`
- [X] Updates to schema generator v2.0
2022-04-17 08:02:15 -07:00
Kamron Batman
17f8d3563a
fix: Codegens treasure/paragon chests & strongboxes (#948) 2022-02-27 17:22:38 -08:00
Kamron Batman
9b554f69b0
feat(timers): Adds timer pooling, fixes timer related bugs, and changes timer api (#667)
### Changes/Fixes:
* Adds timer pooling.
* Allows pool to be configurable in ModernUO.json
* Pool replenishes itself asynchronously if depleted.
* Fixes an issue with barkeeps and town criers
* Fixes an issue with incognito buff icons not being removed
* Fixes an issue with polymorph name mod not being removed
* Fixes several places where timers go on forever even after an object is deleted, keeping a reference (memory leak)
* Eliminates the timer for MiningCart altogether.
* Deletes `AcidSlime` since it is a duplicate of `PoolOfAcid`
* Fixes HonorableExecution and standardizes the code for other Bushido moves.

## Changes to the Timer API:
```cs
public class Timer
{
  // Creates a timer that will be returned to the pool once execution stops.
  public static void StartTimer(Action callback);
  public static void StartTimer(TimeSpan delay, Action callback);
  public static void StartTimer(TimeSpan delay, TimeSpan interval, Action callback);
  public static void StartTimer(TimeSpan interval, int count, Action callback);
  public static void StartTimer(TimeSpan delay, TimeSpan interval, int count, Action callback);

  // Creates a timer and returns a token for more control. Requires manual cancellation in order for the timer to be returned to the pool.
  // If the token is dereferenced, the timer will be dereferenced too. While not returning a timer to the pool is not considered hazardous, it does defeat the purpose of pooled timers.
  public static void StartTimer(Action callback, out TimerExecutionToken token);
  public static void StartTimer(TimeSpan delay, Action callback, out TimerExecutionToken token);
  public static void StartTimer(TimeSpan delay, TimeSpan interval, Action callback, out TimerExecutionToken token);
  public static void StartTimer(TimeSpan interval, int count, Action callback, out TimerExecutionToken token);
  public static void StartTimer(TimeSpan delay, TimeSpan interval, int count, Action callback, out TimerExecutionToken token);

  // If you aren't sure how to use the API above, or you don't care about performance, then you can use the old RunUO Timer.DelayCall
  public static DelayCallTimer DelayCall(Action callback);
  public static DelayCallTimer DelayCall(TimeSpan delay, Action callback);
  public static DelayCallTimer DelayCall(TimeSpan delay, TimeSpan interval, Action callback);
  public static DelayCallTimer DelayCall(TimeSpan interval, int count, Action callback);
  public static DelayCallTimer DelayCall(TimeSpan delay, TimeSpan interval, int count, Action callback);
}

public struct TimerExecutionToken
{
  public bool Running { get; }
  public int Index { get; }
  public int RemainingCount { get; }
  public DateTime Next { get; }
}
```

## When to use `TimerExecutionToken`?
Use tokens when you want to gain the performance benefit of using a pooled timer, but you need one of the following:
* Access to the next time the timer will tick:`token.Next`
* Access to which interval, how many intervals there are, or how many are remaining: `token.Index`, `token.Count`, and `token.RemainingCount`
* Stop a timer manually.
* Determine if the timer is running: `timer.Running`
* See notes below about requirements for using tokens!

## Notes about using the TimerExecutionToken:
When you opt-in to receive a token, you must call `Cancel()` to return the timer. This can be done inside of the callback, or outside of the callback at any time.
If this is not called and your timer is an infinite interval, then you will create a potential memory leak, or null pointer exception in your callback.
If the timer ends and is stopped, but cancel is not called, then the timer will never return to the pool and stay referenced until the token is deleted or cancel is called. (Memory leak)

## Is this thread safe?
No. The ModernUO timer system is not thread safe at all. If you require a thread safe timer system, contact me and I'll help adapt this system. Keep in mind that there is a massive performance hit to make this thread safe when there are literally no use cases for it.

If you need to synchronize execution, meaning you want to execute code from another thread on the core thread. Let's say you have a discord bot that is pushing commands to the game server. Then use `EventLoopContext.Post(SendOrPostCallback callback, object state);`.
2021-08-07 14:33:35 -07:00
Kamron Batman
4ddb3de026
fix(core): Fixes several serialization issues (#355)
- [X] Fixes an issue where a buffer smaller than 8 bytes would not double with enough space in some cases.
- [X] Fixes an issue with dupe copying the savebuffer reference (ugh).
- [X] Streamlines the IGenericWriter API to use better generics.
- [X] Streamlines the IGenericReader API to use better generics.
- [X] Forces `tidying` of a List/HashSet to be done externally since Writers/Readers should not have side effects.
- [X] Fixes an issue where Tidying a list didn't TrimExcess, causing memory leaks.
- [X] Reverted the meaning of `World.Running` to specifically refer to any world state post world loading.
  - NOTE: Do not use this if you want to block on world saves. Instead use checks against `WorldState.Saving` states.
- [X] Fixes an issue with serializing negative DateTime deltas.
- [X] Fixes a potential issue with serializing non-UTC DateTime.

Bumps release version
2020-12-23 07:11:41 -08:00
Kamron Batman
4d6e584b6c
Removes literal variables (#257) 2020-09-18 18:41:26 -07:00
Kamron Batman
8149620b0c
Fixes brace style (#248) 2020-09-13 21:49:46 -07:00
Kamron Batman
ad3775c4d7
Formats UO Content (#201) 2020-08-27 18:30:38 -07:00
Kamron Batman
8ec166bcd0
Adds assemblies config, fixes crash bugs. (#134) 2020-05-09 12:55:56 -07:00
Renamed from Projects/Scripts/Items/Containers/Strongbox.cs (Browse further)