Pen permanently stuck on sign/stamp

[avosirenfal]

Description

The dropdown for my pen doesn't let me select the 'write' option. Every round I start on stamp, and there's no way to change it. This has been going on for a while.

Reproduction
Unclear how/why it started.

Screenshots
Video included.

Additional context
It says I stamped in the floating emote text, but the dropdown on the UI says sign. I'm not sure if that's normal or not. This did start on a round I was trying out stamps for the first time. This happens only with that particular option, not any other dropdowns. I have tried it at 100% UI scale (automatic) instead of 150% but it's still broken.

pen.mp4

[blueDev2]

Pretty sure once you stamp a piece of paper, you can't write on in anymore. So if you attempt to write on it, it will just sign instead.

[exincore]

The dropdown for the pen shows the 'write' option is already selected; try clicking on 'sign' and you'll see it switch to sign. Stamped papers cannot be written to or modified, so clicking on the stamped paper even with the 'write' option will fall back to signing it.

However, it probably should say 'sign' instead of 'stamp' in the popup message.

[avosirenfal]

It's very unintuitive to me that it would sign when it's not set to sign! This wasn't clear to an admin I spoke to before making the ticket either.

Perhaps it should simply reject writing on the paper if the paper has been signed but the pen is set to write. Could signing be made into an alt+click alternative action for convenience?

[exincore]

I think you are right. Looking at the code, I think that behavior is due to an oversight. It seems to be a little tricky to make a pen act as a pen and a stamp while avoiding modifying upstream code.

[avosirenfal]

Wouldn't this do?

diff --git a/Content.Server/Paper/PaperSystem.cs b/Content.Server/Paper/PaperSystem.cs
index 7907c2db..ac5e46d6 100644
--- a/Content.Server/Paper/PaperSystem.cs
+++ b/Content.Server/Paper/PaperSystem.cs
@@ -114,11 +114,20 @@ private void OnInteractUsing(EntityUid uid, PaperComponent paperComp, InteractUs
         {
             // only allow editing if there are no stamps or when using a cyberpen
             var editable = paperComp.StampedBy.Count == 0 || _tagSystem.HasTag(args.Used, "WriteIgnoreStamps");
-            if (_tagSystem.HasTag(args.Used, "Write") && editable)
+            if (_tagSystem.HasTag(args.Used, "Write"))
             {
+                // pen on sign mode will fall through to stamp below
                 if (TryComp<PenComponent>(args.Used, out var penComp) && penComp.Pen == PenMode.PenSign);
                 else // Frontier - Else the rest
                 {
+                    // a pen in write mode cannot edit a paper which has
+                    // already been stamped
+                    if(!editable) {
+                        // TODO: emit some kind of message to the user that
+                        // this action is not allowed?
+                        return;
+                    }
+
                     var writeEvent = new PaperWriteEvent(uid, args.User);
                     RaiseLocalEvent(args.Used, ref writeEvent);
                     if (!TryComp<ActorComponent>(args.User, out var actor))

[exincore]

Maybe. The code path is getting hard to follow and should probably be refactored. There's also some goofy stuff like this

frontier-station-14/Content.Server/Paper/PaperSystem.cs

Lines 245 to 249 in bc6bbc6

	if (stampComp.StampedPersonal)
	{
	if (stampComp.StampedPersonal) // Frontier
	stampComp.StampedName = Loc.GetString("stamp-component-signee-name", ("user", args.User)); // Frontier
	}

[avosirenfal]

Sure, do you like this better? I haven't worked with SS14 before, but I'd be happy to start a PR if this is moving in the right direction.

I don't know what makes StampedPersonal goofy. The fact that the localization string is the variable with no text?

private void sendLocalMessage(EntityUid user, String? selfMsg, String? otherMsg) {
	if(otherMsg != null)
	{
		_popupSystem.PopupEntity(otherMsg, user, Filter.PvsExcept(user, entityManager: EntityManager), true);
	}
	
	if(selfMsg != null)
	{
		_popupSystem.PopupEntity(selfMsg, user, user);
	};
}

private void OnInteractUsing(EntityUid uid, PaperComponent paperComp, InteractUsingEvent args)
{
	// can't write or sign without a pen!
	if (!TryComp<PenComponent>(args.Used, out var penComp))
		return;
	
	// if we're trying to write on the paper
	if (_tagSystem.HasTag(args.Used, "Write") && penComp.Pen == PenMode.PenWrite)
	{
		// paper can only be edited if it hasn't been stamped yet, or using a cyberpen
		var editable = paperComp.StampedBy.Count == 0 || _tagSystem.HasTag(args.Used, "WriteIgnoreStamps");
		
		if(!editable) {
			// paper was already stamped, tell the user they can't write on it
			this.sendLocalMessage(
				args.User,
				Loc.GetString(
					"paper-component-action-stamp-paper-cantwrite",
					("target", args.Target)
				),
				null
			);
			return;
		}

		var writeEvent = new PaperWriteEvent(uid, args.User);
		RaiseLocalEvent(args.Used, ref writeEvent);
		
		if (!TryComp<ActorComponent>(args.User, out var actor))
			return;

		paperComp.Mode = PaperAction.Write;
		_uiSystem.TryOpen(uid, PaperUiKey.Key, actor.PlayerSession);
		UpdateUserInterface(uid, paperComp, actor.PlayerSession);
		args.Handled = true;
		return;
	// sign the paper
	} else if (penComp.Pen == PenMode.PenSign) {
		// attempt to stamp
		if (!TryComp<StampComponent>(args.Used, out var stampComp) || !TryStamp(uid, GetStampInfo(stampComp), stampComp.StampState, paperComp))
			return;
		
		if (stampComp.StampedPersonal) // Frontier
			stampComp.StampedName = Loc.GetString("stamp-component-signee-name", ("user", args.User)); // Frontier
			
		// successfully stamped, play popup
		this.sendLocalMessage(
			args.User,
			Loc.GetString(
				"paper-component-action-stamp-paper-self",
				("target", args.Target),
				("stamp", args.Used)
			),
			Loc.GetString(
				"paper-component-action-stamp-paper-other",
				("user", args.User),
				("target", args.Target),
				("stamp", args.Used)
			)
		)

		_audio.PlayPvs(stampComp.Sound, uid);
		UpdateUserInterface(uid, paperComp);
	}
}

[exincore]

That might work for pens, but remember stamps use the same code, so the TryComp<PenComponent> guard at the beginning will break normal stamps.

[exincore]

If you want to make a PR on this then go ahead.

[exincore]

You might want to take a look at what Delta-V just did is doing relating to pen signing. It looks less convoluted than what we have now.

DeltaV-Station/Delta-v#1172

[dvir001]

Try to sign again after upstream update I found the issue.

[dvir001]

Fixed the issue on upstream merge that coming soon.