- implement flag variables with the VM's sbit and lbit instructions.

- synthesize PField entries from the flag list for AActor. This intentionally excludes the bounce flags for now.
- allow deprecated flags that do not call the deprecated flag handler.
- disallow constructs like (a = b) = c by not allowing an address request on an assignment operation.
- restrict modify/assign on boolean variables to the bit operators. Everything else needs to promote the result to an integer to make sense so it should be disallowed.

src/dobjtype.cpp

@@ -2479,6 +2479,31 @@ PField::PField()
 {
 }
 
+PField::PField(FName name, PType *type, DWORD flags, size_t offset, int bitvalue)
+	: PSymbol(name), Offset(unsigned(offset)), Type(type), Flags(flags)
+{
+	BitValue = bitvalue;
+	if (bitvalue > -1)
+	{
+		if (type->IsA(RUNTIME_CLASS(PInt)) && unsigned(bitvalue) < 8 * type->Size)
+		{
+			// map to the single bytes in the actual variable. The internal bit instructions operate on 8 bit values.
+#ifndef __BIG_ENDIAN__
+			Offset += BitValue / 8;
+#else
+			Offset += type->Size - 1 - BitValue / 8;
+#endif
+			BitValue &= 7;
+			Type = TypeBool;
+		}
+		else
+		{
+			// Just abort. Bit fields should only be defined internally.
+			I_FatalError("Trying to create an invalid bit field element: %s", name.GetChars());
+		}
+	}
+}
+
 /* PPrototype *************************************************************/
 
 IMPLEMENT_CLASS(PPrototype)

src/dobjtype.h

@@ -572,13 +572,12 @@ class PField : public PSymbol
 	DECLARE_CLASS(PField, PSymbol);
 	HAS_OBJECT_POINTERS
 public:
-	PField(FName name, PType *type) : PSymbol(name), Offset(0), Type(type), Flags(0) {}
+	PField(FName name, PType *type, DWORD flags = 0, size_t offset = 0, int bitvalue = -1);
-	PField(FName name, PType *type, DWORD flags) : PSymbol(name), Offset(0), Type(type), Flags(flags) {}
-	PField(FName name, PType *type, DWORD flags, size_t offset) : PSymbol(name), Offset(unsigned(offset)), Type(type), Flags(flags) {}
 
 	unsigned int Offset;
 	PType *Type;
 	DWORD Flags;
+	int BitValue;
 protected:
 	PField();
 };

src/scripting/codegeneration/codegen.cpp

@@ -1768,8 +1768,8 @@ ExpEmit FxPostIncrDecr::Emit(VMFunctionBuilder *build)
 //
 //==========================================================================
 
-FxAssign::FxAssign(FxExpression *base, FxExpression *right)
+FxAssign::FxAssign(FxExpression *base, FxExpression *right, bool ismodify)
-: FxExpression(EFX_Assign, base->ScriptPosition), Base(base), Right(right)
+: FxExpression(EFX_Assign, base->ScriptPosition), Base(base), Right(right), IsModifyAssign(ismodify), IsBitWrite(-1)
 {
 	AddressRequested = false;
 	AddressWritable = false;
@@ -1781,12 +1781,14 @@ FxAssign::~FxAssign()
 	SAFE_DELETE(Right);
 }
 
+/* I don't think we should allow constructs like (a = b) = c;...
 bool FxAssign::RequestAddress(bool *writable)
 {
 	AddressRequested = true;
 	if (writable != nullptr) *writable = AddressWritable;
 	return true;
 }
+*/
 
 FxExpression *FxAssign::Resolve(FCompileContext &ctx)
 {
@@ -1797,6 +1799,15 @@ FxExpression *FxAssign::Resolve(FCompileContext &ctx)
 
 	SAFE_RESOLVE(Right, ctx);
 
+	if (IsModifyAssign && Base->ValueType == TypeBool && Right->ValueType != TypeBool)
+	{
+		// If the modify operation resulted in a type promotion from bool to int, this must be blocked.
+		// (this means, for bool, only &=, ^= and |= are allowed, although DECORATE is more lax.)
+		ScriptPosition.Message(MSG_ERROR, "Invalid modify/assign operation with a boolean operand");
+		delete this;
+		return nullptr;
+	}
+
 	if (Base->IsNumeric() && Right->IsNumeric())
 	{
 		if (Right->ValueType != ValueType)
@@ -1865,6 +1876,16 @@ FxExpression *FxAssign::Resolve(FCompileContext &ctx)
 		return nullptr;
 	}
 
+	// Special case: Assignment to a bitfield.
+	if (Base->ExprType == EFX_StructMember || Base->ExprType == EFX_ClassMember)
+	{
+		auto f = static_cast<FxStructMember *>(Base)->membervar;
+		if (f->BitValue != -1 && !(f->Flags & VARF_ReadOnly))
+		{
+			IsBitWrite = f->BitValue;
+			return this;
+		}
+	}
 
 	return this;
 }
@@ -1902,8 +1923,16 @@ ExpEmit FxAssign::Emit(VMFunctionBuilder *build)
 			result = temp;
 		}
 
+		if (IsBitWrite == -1)
+		{
 			build->Emit(ValueType->GetStoreOp(), pointer.RegNum, result.RegNum, build->GetConstantInt(0));
 		}
+		else
+		{
+			build->Emit(OP_SBIT, pointer.RegNum, result.RegNum, 1 << IsBitWrite);
+		}
+
+	}
 
 	if (AddressRequested)
 	{
@@ -1941,12 +1970,19 @@ FxExpression *FxAssignSelf::Resolve(FCompileContext &ctx)
 
 ExpEmit FxAssignSelf::Emit(VMFunctionBuilder *build)
 {
-	assert(ValueType = Assignment->ValueType);
+	assert(ValueType == Assignment->ValueType);
 	ExpEmit pointer = Assignment->Address; // FxAssign should have already emitted it
 	if (!pointer.Target)
 	{
 		ExpEmit out(build, ValueType->GetRegType());
+		if (Assignment->IsBitWrite != -1)
+		{
+			build->Emit(OP_LBIT, out.RegNum, pointer.RegNum, 1 << Assignment->IsBitWrite);
+		}
+		else
+		{
 			build->Emit(ValueType->GetLoadOp(), out.RegNum, pointer.RegNum, build->GetConstantInt(0));
+		}
 		return out;
 	}
 	else
@@ -1998,12 +2034,30 @@ bool FxBinary::ResolveLR(FCompileContext& ctx, bool castnumeric)
 	}
 
 	if (left->ValueType == TypeBool && right->ValueType == TypeBool)
+	{
+		if (Operator == '&' || Operator == '|' || Operator == '^' || ctx.FromDecorate)
 		{
 			ValueType = TypeBool;
 		}
+		else
+		{
+			ValueType = TypeSInt32;	// math operations on bools result in integers.
+		}
+	}
 	else if (left->ValueType == TypeName && right->ValueType == TypeName)
 	{
-		ValueType = TypeName;
+		// pointers can only be compared for equality.
+		if (Operator == TK_Eq || Operator == TK_Neq)
+		{
+			ValueType = TypeBool;
+			return true;
+		}
+		else
+		{
+			ScriptPosition.Message(MSG_ERROR, "Invalid operation for names");
+			delete this;
+			return false;
+		}
 	}
 	else if (left->IsNumeric() && right->IsNumeric())
 	{
@@ -2022,13 +2076,25 @@ bool FxBinary::ResolveLR(FCompileContext& ctx, bool castnumeric)
 			AreCompatiblePointerTypes(left->ValueType, right->ValueType))
 		{
 			// pointers can only be compared for equality.
-			assert(Operator == TK_Eq || Operator == TK_Neq);
+			if (Operator == TK_Eq || Operator == TK_Neq)
+			{
 				ValueType = TypeBool;
+				return true;
+			}
+			else
+			{
+				ScriptPosition.Message(MSG_ERROR, "Invalid operation for pointers");
+				delete this;
+				return false;
+			}
 		}
 	}
 	else
 	{
-		ValueType = TypeVoid;
+		// To check: It may be that this could pass in DECORATE, although setting TypeVoid here would pretty much prevent that.
+		ScriptPosition.Message(MSG_ERROR, "Incompatible operator");
+		delete this;
+		return false;
 	}
 	assert(ValueType > nullptr && ValueType < (PType*)0xfffffffffffffff);
 
@@ -2077,7 +2143,7 @@ FxExpression *FxAddSub::Resolve(FCompileContext& ctx)
 	{
 		ScriptPosition.Message(MSG_ERROR, "Numeric type expected");
 		delete this;
-		return NULL;
+		return nullptr;
 	}
 	else if (left->isConstant() && right->isConstant())
 	{
@@ -2771,7 +2837,7 @@ FxLtGtEq::FxLtGtEq(FxExpression *l, FxExpression *r)
 FxExpression *FxLtGtEq::Resolve(FCompileContext& ctx)
 {
 	CHECKRESOLVED();
-	if (!ResolveLR(ctx, false)) return NULL;
+	if (!ResolveLR(ctx, true)) return NULL;
 
 	if (!left->IsNumeric() || !right->IsNumeric())
 	{
@@ -4456,21 +4522,21 @@ FxExpression *FxStructMember::Resolve(FCompileContext &ctx)
 	if (classx->ValueType->IsKindOf(RUNTIME_CLASS(PPointer)))
 	{
 		PPointer *ptrtype = dyn_cast<PPointer>(classx->ValueType);
-		if (ptrtype == nullptr || !ptrtype->PointedType->IsA(RUNTIME_CLASS(PStruct)))
+		if (ptrtype == nullptr || !ptrtype->PointedType->IsKindOf(RUNTIME_CLASS(PStruct)))
 		{
-			ScriptPosition.Message(MSG_ERROR, "Member variable requires a struct");
+			ScriptPosition.Message(MSG_ERROR, "Member variable requires a struct or class object.");
 			delete this;
 			return nullptr;
 		}
 	}
-	else if (classx->ValueType->IsA(RUNTIME_CLASS(PStruct)))
+	else if (classx->ValueType->IsA(RUNTIME_CLASS(PStruct)))	// Classes can never be used as value types so we do not have to consider that case.
 	{
 		// if this is a struct within a class or another struct we can simplify the expression by creating a new PField with a cumulative offset.
 		if (classx->ExprType == EFX_ClassMember || classx->ExprType == EFX_StructMember)
 		{
 			auto parentfield = static_cast<FxStructMember *>(classx)->membervar;
 			// PFields are garbage collected so this will be automatically taken care of later.
-			auto newfield = new PField(membervar->SymbolName, membervar->Type, membervar->Flags | parentfield->Flags, membervar->Offset + parentfield->Offset);
+			auto newfield = new PField(membervar->SymbolName, membervar->Type, membervar->Flags | parentfield->Flags, membervar->Offset + parentfield->Offset, membervar->BitValue);
 			static_cast<FxStructMember *>(classx)->membervar = newfield;
 			classx->isresolved = false;	// re-resolve the parent so it can also check if it can be optimized away.
 			auto x = classx->Resolve(ctx);
@@ -4522,7 +4588,17 @@ ExpEmit FxStructMember::Emit(VMFunctionBuilder *build)
 	int offsetreg = build->GetConstantInt((int)membervar->Offset);
 	ExpEmit loc(build, membervar->Type->GetRegType());
 
+	if (membervar->BitValue == -1)
+	{
 		build->Emit(membervar->Type->GetLoadOp(), loc.RegNum, obj.RegNum, offsetreg);
+	}
+	else
+	{
+		ExpEmit out(build, REGT_POINTER);
+		build->Emit(OP_ADDA_RK, out.RegNum, obj.RegNum, offsetreg);
+		build->Emit(OP_LBIT, loc.RegNum, out.RegNum, 1 << membervar->BitValue);
+		out.Free(build);
+	}
 	obj.Free(build);
 	return loc;
 }
@@ -4530,7 +4606,8 @@ ExpEmit FxStructMember::Emit(VMFunctionBuilder *build)
 
 //==========================================================================
 //
-//
+// not really needed at the moment but may become useful with meta properties
+// and some other class-specific extensions.
 //
 //==========================================================================
 
@@ -4547,28 +4624,6 @@ FxClassMember::FxClassMember(FxExpression *x, PField* mem, const FScriptPosition
 //
 //==========================================================================
 
-FxExpression *FxClassMember::Resolve(FCompileContext &ctx)
-{
-	CHECKRESOLVED();
-	SAFE_RESOLVE(classx, ctx);
-
-	PPointer *ptrtype = dyn_cast<PPointer>(classx->ValueType);
-	if (ptrtype == NULL || !ptrtype->PointedType->IsKindOf(RUNTIME_CLASS(PClass)))
-	{
-		ScriptPosition.Message(MSG_ERROR, "Member variable requires a class or object");
-		delete this;
-		return NULL;
-	}
-	ValueType = membervar->Type;
-	return this;
-}
-
-//==========================================================================
-//
-//
-//
-//==========================================================================
-
 FxArrayElement::FxArrayElement(FxExpression *base, FxExpression *_index)
 :FxExpression(EFX_ArrayElement, base->ScriptPosition)
 {

src/scripting/codegeneration/codegen.h

@@ -703,14 +703,18 @@ class FxAssign : public FxExpression
 {
 	FxExpression *Base;
 	FxExpression *Right;
+	int IsBitWrite;
 	bool AddressRequested;
 	bool AddressWritable;
+	bool IsModifyAssign;
+
+	friend class FxAssignSelf;
 
 public:
-	FxAssign(FxExpression *base, FxExpression *right);
+	FxAssign(FxExpression *base, FxExpression *right, bool ismodify = false);
 	~FxAssign();
 	FxExpression *Resolve(FCompileContext&);
-	bool RequestAddress(bool *writable);
+	//bool RequestAddress(bool *writable);
 	ExpEmit Emit(VMFunctionBuilder *build);
 
 	ExpEmit Address;
@@ -1064,7 +1068,6 @@ class FxClassMember : public FxStructMember
 public:
 
 	FxClassMember(FxExpression*, PField*, const FScriptPosition&);
-	FxExpression *Resolve(FCompileContext&);
 };
 
 //==========================================================================

src/scripting/decorate/thingdef_exp.cpp

@@ -170,7 +170,7 @@ static FxExpression *ParseExpressionM (FScanner &sc, PClassActor *cls)
 
 		exp->right = ParseExpressionM(sc, cls);
 
-		FxAssign *ret = new FxAssign(base, exp);
+		FxAssign *ret = new FxAssign(base, exp, true);
 		left->Assignment = ret;
 		return ret;
 	}

src/scripting/thingdef.h

@@ -24,6 +24,7 @@ struct FFlagDef
 	const char *name;
 	int structoffset;
 	int fieldsize;
+	bool deprecated;
 };
 
 FFlagDef *FindFlag (const PClass *type, const char *part1, const char *part2, bool strict = false);

src/scripting/thingdef_data.cpp

@@ -54,10 +54,11 @@ static TArray<AFuncDesc> AFTable;
 //==========================================================================
 
 // [RH] Keep GCC quiet by not using offsetof on Actor types.
-#define DEFINE_FLAG(prefix, name, type, variable) { (unsigned int)prefix##_##name, #name, (int)(size_t)&((type*)1)->variable - 1, sizeof(((type *)0)->variable) }
+#define DEFINE_FLAG(prefix, name, type, variable) { (unsigned int)prefix##_##name, #name, (int)(size_t)&((type*)1)->variable - 1, sizeof(((type *)0)->variable), false }
-#define DEFINE_FLAG2(symbol, name, type, variable) { (unsigned int)symbol, #name, (int)(size_t)&((type*)1)->variable - 1, sizeof(((type *)0)->variable) }
+#define DEFINE_FLAG2(symbol, name, type, variable) { (unsigned int)symbol, #name, (int)(size_t)&((type*)1)->variable - 1, sizeof(((type *)0)->variable), false }
-#define DEFINE_DEPRECATED_FLAG(name) { DEPF_##name, #name, -1, 0 }
+#define DEFINE_FLAG2_DEPRECATED(symbol, name, type, variable) { (unsigned int)symbol, #name, (int)(size_t)&((type*)1)->variable - 1, sizeof(((type *)0)->variable), true }
-#define DEFINE_DUMMY_FLAG(name) { DEPF_UNUSED, #name, -1, 0 }
+#define DEFINE_DEPRECATED_FLAG(name) { DEPF_##name, #name, -1, 0, true }
+#define DEFINE_DUMMY_FLAG(name, deprec) { DEPF_UNUSED, #name, -1, 0, deprec }
 
 static FFlagDef ActorFlagDefs[]=
 {
@@ -171,7 +172,6 @@ static FFlagDef ActorFlagDefs[]=
 	DEFINE_FLAG(MF4, SYNCHRONIZED, AActor, flags4),
 	DEFINE_FLAG(MF4, NOTARGETSWITCH, AActor, flags4),
 	DEFINE_FLAG(MF4, DONTHARMCLASS, AActor, flags4),
-	DEFINE_FLAG2(MF4_DONTHARMCLASS, DONTHURTSPECIES, AActor, flags4), // Deprecated name as an alias
 	DEFINE_FLAG(MF4, SHIELDREFLECT, AActor, flags4),
 	DEFINE_FLAG(MF4, DEFLECT, AActor, flags4),
 	DEFINE_FLAG(MF4, ALLOWPARTICLES, AActor, flags4),
@@ -278,7 +278,11 @@ static FFlagDef ActorFlagDefs[]=
 	DEFINE_FLAG(RF, MASKROTATION, AActor, renderflags),
 	DEFINE_FLAG(RF, ABSMASKANGLE, AActor, renderflags),
 	DEFINE_FLAG(RF, ABSMASKPITCH, AActor, renderflags),
+};
 
+// These won't be accessible through bitfield variables
+static FFlagDef MoreFlagDefs[] =
+{
 	// Bounce flags
 	DEFINE_FLAG2(BOUNCE_Walls, BOUNCEONWALLS, AActor, BounceFlags),
 	DEFINE_FLAG2(BOUNCE_Floors, BOUNCEONFLOORS, AActor, BounceFlags),
@@ -308,15 +312,16 @@ static FFlagDef ActorFlagDefs[]=
 	DEFINE_DEPRECATED_FLAG(DOOMBOUNCE),
 
 	// Deprecated flags with no more existing functionality.
-	DEFINE_DUMMY_FLAG(FASTER),				// obsolete, replaced by 'Fast' state flag
+	DEFINE_DUMMY_FLAG(FASTER, true),				// obsolete, replaced by 'Fast' state flag
-	DEFINE_DUMMY_FLAG(FASTMELEE),			// obsolete, replaced by 'Fast' state flag
+	DEFINE_DUMMY_FLAG(FASTMELEE, true),			// obsolete, replaced by 'Fast' state flag
 
 	// Various Skulltag flags that are quite irrelevant to ZDoom
-	DEFINE_DUMMY_FLAG(NONETID),				// netcode-based
+	DEFINE_DUMMY_FLAG(NONETID, false),				// netcode-based
-	DEFINE_DUMMY_FLAG(ALLOWCLIENTSPAWN),	// netcode-based
+	DEFINE_DUMMY_FLAG(ALLOWCLIENTSPAWN, false),	// netcode-based
-	DEFINE_DUMMY_FLAG(CLIENTSIDEONLY),	    // netcode-based
+	DEFINE_DUMMY_FLAG(CLIENTSIDEONLY, false),	    // netcode-based
-	DEFINE_DUMMY_FLAG(SERVERSIDEONLY),		// netcode-based
+	DEFINE_DUMMY_FLAG(SERVERSIDEONLY, false),		// netcode-based
-	DEFINE_DUMMY_FLAG(EXPLODEONDEATH),	    // seems useless
+	DEFINE_DUMMY_FLAG(EXPLODEONDEATH, true),	    // seems useless
+	DEFINE_FLAG2_DEPRECATED(MF4_DONTHARMCLASS, DONTHURTSPECIES, AActor, flags4), // Deprecated name as an alias
 };
 
 static FFlagDef InventoryFlagDefs[] =
@@ -372,9 +377,9 @@ static FFlagDef WeaponFlagDefs[] =
 	DEFINE_FLAG(WIF, NODEATHDESELECT, AWeapon, WeaponFlags),
 	DEFINE_FLAG(WIF, NODEATHINPUT, AWeapon, WeaponFlags),
 	
-	DEFINE_DUMMY_FLAG(NOLMS),
+	DEFINE_DUMMY_FLAG(NOLMS, false),
 	DEFINE_FLAG(WIF, ALT_USES_BOTH, AWeapon, WeaponFlags),
-	DEFINE_DUMMY_FLAG(ALLOW_WITH_RESPAWN_INVUL),
+	DEFINE_DUMMY_FLAG(ALLOW_WITH_RESPAWN_INVUL, false),
 };
 
 static FFlagDef PlayerPawnFlagDefs[] =
@@ -394,6 +399,7 @@ static FFlagDef PowerSpeedFlagDefs[] =
 static const struct FFlagList { const PClass * const *Type; FFlagDef *Defs; int NumDefs; } FlagLists[] =
 {
 	{ &RUNTIME_CLASS_CASTLESS(AActor), 		ActorFlagDefs,		countof(ActorFlagDefs) },
+	{ &RUNTIME_CLASS_CASTLESS(AActor), 		MoreFlagDefs,		countof(MoreFlagDefs) },
 	{ &RUNTIME_CLASS_CASTLESS(AInventory), 	InventoryFlagDefs,	countof(InventoryFlagDefs) },
 	{ &RUNTIME_CLASS_CASTLESS(AWeapon), 	WeaponFlagDefs,		countof(WeaponFlagDefs) },
 	{ &RUNTIME_CLASS_CASTLESS(APlayerPawn),	PlayerPawnFlagDefs,	countof(PlayerPawnFlagDefs) },
@@ -443,7 +449,7 @@ FFlagDef *FindFlag (const PClass *type, const char *part1, const char *part2, bo
 
 	if (part2 == NULL)
 	{ // Search all lists
-		int max = strict ? 1 : NUM_FLAG_LISTS;
+		int max = strict ? 2 : NUM_FLAG_LISTS;
 		for (i = 0; i < max; ++i)
 		{
 			if (type->IsDescendantOf (*FlagLists[i].Type))
@@ -683,4 +689,14 @@ void InitThingdef()
 	symt.AddSymbol(new PField(NAME_Target,			TypeActor,		VARF_Native,				myoffsetof(AActor, target)));
 	symt.AddSymbol(new PField(NAME_Master,			TypeActor,		VARF_Native,				myoffsetof(AActor, master)));
 	symt.AddSymbol(new PField(NAME_Tracer,			TypeActor,		VARF_Native,				myoffsetof(AActor, tracer)));
+
+	// synthesize a symbol for each flag. The bounce flags are excluded on purpose.
+	for (size_t i = 0; i < countof(ActorFlagDefs); i++)
+	{
+		int bit = 0;
+		unsigned val = ActorFlagDefs[i].flagbit;
+		while ((val >>= 1)) bit++;
+		symt.AddSymbol(new PField(FStringf("b%s", ActorFlagDefs[i].name), (ActorFlagDefs[i].fieldsize == 4? TypeSInt32 : TypeSInt16), VARF_Native, ActorFlagDefs[i].structoffset, bit));
+	}
+
 }

src/scripting/zscript/zcc_compile.cpp

@@ -1836,9 +1836,12 @@ void ZCCCompiler::ProcessDefaultFlag(PClassActor *cls, ZCC_FlagStmt *flg)
 	auto fd = FindFlag(cls, n1, n2, true);
 	if (fd != nullptr)
 	{
-		if (fd->structoffset == -1)
+		if (fd->deprecated)
 		{
 			Warn(flg, "Deprecated flag '%s%s%s' used", n1, n2 ? "." : "", n2 ? n2 : "");
+		}
+		if (fd->structoffset == -1)
+		{
 			HandleDeprecatedFlags((AActor*)cls->Defaults, cls, flg->set, fd->flagbit);
 		}
 		else
@@ -2384,7 +2387,7 @@ static int Pex2Tok[] = {
 static FxExpression *ModifyAssign(FxBinary *operation, FxExpression *left)
 {
 	auto assignself = static_cast<FxAssignSelf *>(operation->left);
-	auto assignment = new FxAssign(left, operation);
+	auto assignment = new FxAssign(left, operation, true);
 	assignself->Assignment = assignment;
 	return assignment;
 }