fix: Require BOD combine items to be player-crafted (#2573)

## Problem

`SmallBOD.EndCombine` validates an item's **type**, **material** and **exceptional quality**, but never checks that the item was actually crafted by a player. Any item matching the request is accepted, including one bought straight from an NPC vendor.

https://github.com/modernuo/ModernUO/blob/main/Projects/UOContent/Engines/Bulk%20Orders/SmallBOD.cs#L117-L168

Where a vendor stocks a type a BOD can request, a player can fill the deed by buying the items instead of crafting them, and pocket the reward gold for the difference.

Tailoring is the clearest case. `SmallTailorBOD.CreateRandomFor` guarantees `Material = None` and `RequireExceptional = false` below 70.1 skill, so the rolled deed asks for plain cloth items — and tailor vendors stock several of those directly. A qty-20 Bandana BOD can be filled entirely from vendor stock for a small fraction of the reward gold, with no crafting and no material cost.

The same shape applies anywhere else a vendor-sold type overlaps a requestable BOD type; tailoring is simply where the low-skill deed generator and the vendor inventory overlap most.

## Fix

Add a `PlayerConstructed` check alongside the existing material and quality checks.

```csharp
var playerConstructed = armor?.PlayerConstructed ?? clothing?.PlayerConstructed ??
    weapon?.PlayerConstructed ?? false;

if (!playerConstructed)
{
    from.SendLocalizedMessage(1045169); // The item is not in the request.
}
```

This follows the pattern already used in `Engines/Craft/Core/Resmelt.cs` (L98-L100, L155-L160) to distinguish crafted from store-bought items, and reuses the same null-coalescing chain style as the adjacent `GetMaterial(armor?.Resource ?? clothing?.Resource ?? CraftResource.None)` line directly above it.

`PlayerConstructed` is already set in `OnCraft` and serialized on all three bases (`BaseArmor`, `BaseWeapon`, `BaseClothing`), so the flag survives restarts and no serialization change is needed.

## Open question — the message

There is no dedicated cliloc for "this item must be crafted", so I reused **1045169** (*"The item is not in the request."*). It is arguably accurate — a vendor-bought item genuinely is not what the deed asked for — but it is not precise, and a player who does not know the rule will find it confusing.

I would rather flag this than invent a string. If there is a better cliloc, I am happy to switch it.

## Testing

`dotnet build Projects/UOContent/UOContent.csproj` — **0 errors, 0 warnings**.

Not covered: I have not added an automated test, as I could not find existing coverage for `EndCombine` to extend. Happy to add one if you would like it, with a pointer to the preferred pattern.

## Compatibility note

Any *already-existing* vendor-bought item in a player's possession will now be rejected by a BOD. That is the intended behaviour, but it is a visible change for anyone mid-deed. Worth a line in release notes.
This commit is contained in:
Kamron Batman 2026-08-12 20:12:54 -07:00 committed by GitHub
parent 1bc83339bb
commit 5ce0f1e92b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -136,8 +136,14 @@ public abstract partial class SmallBOD : BaseBOD
else else
{ {
var material = GetMaterial(armor?.Resource ?? clothing?.Resource ?? CraftResource.None); var material = GetMaterial(armor?.Resource ?? clothing?.Resource ?? CraftResource.None);
var playerConstructed = armor?.PlayerConstructed ?? clothing?.PlayerConstructed ??
weapon?.PlayerConstructed ?? false;
if (Material >= BulkMaterialType.DullCopper && Material <= BulkMaterialType.Valorite && material != Material) if (!playerConstructed)
{
from.SendLocalizedMessage(1045169); // The item is not in the request.
}
else if (Material >= BulkMaterialType.DullCopper && Material <= BulkMaterialType.Valorite && material != Material)
{ {
from.SendLocalizedMessage(1045168); // The item is not made from the requested ore. from.SendLocalizedMessage(1045168); // The item is not made from the requested ore.
} }