Ludeon Forums

Ludeon Forums

  • May 09, 2021, 09:56:39 AM
  • Welcome, Guest
Please login or register.

Login with username, password and session length
Advanced search  

Show Posts

This section allows you to view all posts made by this member. Note that you can only see posts made in areas you currently have access to.

Topics - Alistaire

Pages: [1] 2
1
Setting off several stacks of large-radius explosives is extremely detrimental to performance. Without this fix, the creation of motes provides an incredible RAM throttle. The fix improves performance around hundred-fold during large, consecutive and nearby explosions.

----

The major cause of this is the following:

EACH Explosion spawns damType.explosionCellMote onto EACH tile, since Explosion.AffectCell(IntVec3 c) calls damType.Worker.ExplosionAffectCell(., ., ., ., bool canThrowMotes) with canThrowMotes = true, whenever ShouldCellBeAffectedOnlyByDamage(IntVec3 c) is true.

The ShouldCellBeAffectedOnlyByDamage check is true for nearly every tile.

This means that in a stockpile full of antigrain mortar shells (don't ask), one mortar shell detonates setting off all other mortar shells. Each explosion spawns EXACTLY THE SAME damType.explosionCellMote on every affected cell, meaning EACH CELL contains upwards of the total amount of mortar shell stacks that have exploded.

Creating a mote on each cell is extremely costly, such that setting off a stack of mortar shells can easily lead to <1 FPS framerates.

----

The solution to improving performance significantly:

In AffectCell, add the following boolean, here called spawnMote:

Code: [Select]
var spawnMote = c.GetFirstThing(Map, damType.explosionCellMote) == null;
Then, call the worker with spawnMote && !flag instead of just !flag.

Code: [Select]
this.damType.Worker.ExplosionAffectCell(this, c, this.damagedThings, this.ignoredThings, spawnMote && !flag);
What results is a nearly- N-fold reduction in motes being created per tick, with N the amount of explosions. As a result, framerates throughout massive antigrain explosions become >>1 FPS, for my old computer even up to 15-30 FPS from ~60 seconds per frame for especially large explosions.

2
Bugs / [1.1.2567 (+ Royalty)] QuestNode_Equal compares incorrectly
« on: March 10, 2020, 09:55:28 AM »
Using mods which add new children of Def, the following issue occurs:

  • QuestScriptDef TradeRequest is tested for
  • QuestNode_TradeRequest_Initiate returns $requestedThing which is a child of ThingDef, such as CombatExtended.AmmoDef or any other child of ThingDef
  • QuestNode_Equal tests whether $requestedThing is Leather_Human
  • QuestNodeEqualUtility.Equal(this.value1.GetValue(slate), this.value2.GetValue(slate), this.compareAs.GetValue(slate)), the value of slate.GetValue() is CombatExtended.AmmoDef
  • ConvertHelper.CanConvert returns true because CombatExtended.AmmoDef is a child of ThingDef (IsAssignableFrom)
  • ConvertHelper.Convert is called, which because CombatExtended.AmmoDef is assignable from Def, checks through the DefDatabase of CombatExtended.AmmoDef!
  • Attempt to GetNamed<CombatExtended.AmmoDef>("Leather_Human")
  • Error, because the AmmoDef DefDatabase does not contain a definition for Leather_Human

This error prevents mods from extending ThingDef, just because of a comparison to human leather for no good reason.

There should be a fallback which checks whether the Def children EXACTLY match rather than if they are assignable from eachother, at least in this case. Specifically, the DefDatabase does NOT allow calling one ThingDef and converting it to another Type : ThingDef, so this equality comparison shouldn't be able to do so (e.g it shouldn't check whether the types are assignable from eachother, but whether they ARE eachother).



Since this is an interpretation of the issues; exact stacktrace attached to post (stacktrace.png).

3
This is because Verse.Verb.CanHitTargetFrom checks Verb.CasterIsPawn and thus Verb.caster.

Code: [Select]
// Verse.Verb
public virtual bool CanHitTargetFrom(IntVec3 root, LocalTargetInfo targ)

(..)

if (this.CasterIsPawn && this.CasterPawn.apparel != null)
{

(..)

Verb.caster is set to the turretGun, not to the operator of the mannable turret.

Code: [Select]
// RimWorld.Building_TurretGun
private void UpdateGunVerbs()
{
List<Verb> allVerbs = this.gun.TryGetComp<CompEquippable>().AllVerbs;
for (int i = 0; i < allVerbs.Count; i++)
{
Verb verb = allVerbs[i];
verb.caster = this;
verb.castCompleteCallback = new Action(this.BurstComplete);
}
}

Therefore, the WornApparel of the operator is never checked for Apparel.AllowVerbCast().

Code: [Select]
if (this.CasterIsPawn && this.CasterPawn.apparel != null)
{
List<Apparel> wornApparel = this.CasterPawn.apparel.WornApparel;
for (int i = 0; i < wornApparel.Count; i++)
{
if (!wornApparel[i].AllowVerbCast(root, this.caster.Map, targ, this))
{
return false;
}
}
}

So the following code for shieldbelts is never checked:

Code: [Select]
// RimWorld.ShieldBelt
public override bool AllowVerbCast(IntVec3 root, Map map, LocalTargetInfo targ, Verb verb)
{
return !(verb is Verb_LaunchProjectile) || ReachabilityImmediate.CanReachImmediate(root, targ, map, PathEndMode.Touch, null);
}

Thus pawns can fire mortars and other manned turrets, even though they have Verb_LaunchProjectile and their targets aren't within touch range.

4
Bugs / [0.18.1722] Modding: Tool config error checking
« on: January 01, 2018, 11:54:07 AM »
  • HediffDef.ConfigErrors doesn't check tools, while ThingDef.ConfigErrors does so. A HediffDef with tools is for example PowerClaw
  • ToolCapacityDef aren't checked for having a corresponding ManeuverDef
  • Because of the last, VerbTracker won't recognize a capacity or maneuver (say on a modded item), rendering the HediffDef/ThingDef without a verb. Because there's no feedback from ConfigErrors, bugs such as these can be difficult to find and even more difficult to fix

5
Bugs / [A17b] Untranslatable strings in CompSchedule, CompUsable
« on: June 29, 2017, 05:31:27 AM »
CompProperties_Usable uses a "useLabel" instead of a "useKey", CompProperties_Schedule uses a "offMessage" instead of a "offKey":

Code: [Select]
// RimWorld.CompProperties_Usable
public string useLabel;

// RimWorld.CompProperties_Schedule
public string offMessage;

Which is then displayed as such:

Code: [Select]
// RimWorld.CompUsable
protected virtual string FloatMenuOptionLabel
{
get
{
return this.Props.useLabel;
}
}

// RimWorld.CompSchedule
public override string CompInspectStringExtra()
{
if (!this.Allowed)
{
return this.Props.offMessage;
}
return null;
}

Which means that useLabel/offMessage can not be translated into other languages.



Suggested solution

Code: [Select]
// RimWorld.CompProperties_Usable
public string useKey;

// RimWorld.CompProperties_Schedule
public string offKey;

Code: [Select]
// RimWorld.CompUsable
protected virtual string FloatMenuOptionLabel
{
get
{
return this.Props.useKey.Translate();
}
}

// RimWorld.CompSchedule
public override string CompInspectStringExtra()
{
if (!this.Allowed)
{
return this.Props.offKey.Translate();
}
return null;
}

Code: [Select]
// RimWorld1557Win\Mods\Core\Languages\English\Keyed\Misc_Gameplay.xml

  <!-- Comps -->
  <UseLabel_Neurotrainer>Use neurotrainer to learn {0}</UseLabel_Neurotrainer>
  <UseLabel_Artifact>Activate</UseLabel_Artifact>

  <OffMessage_SunLamp>Off for plant resting period</OffMessage_SunLamp>

6
It appears that props.explosiveExpandPerStackcount has no effect on explosion size. This is because of the following lines:

Code: [Select]
// RimWorld.CompExplosive
///protected void Detonate(Map map)

(..)

if (!this.parent.Destroyed)
{
this.parent.Kill(null);
}

(..)

float num = props.explosiveRadius;
if (this.parent.stackCount > 1 && props.explosiveExpandPerStackcount > 0f)
{
num += Mathf.Sqrt((float)(this.parent.stackCount - 1) * props.explosiveExpandPerStackcount);
}

(..)

Which causes the following to happen. Since this.parent.Kill(null), parent.stackCount = 0. Therefore, regardless of props.explosiveExpandPerStackcount, the explosion radius will be props.explosiveRadius.

Furthermore, there's a check for this.parent further down in the code (which may or may not have problems triggering because of the earlier killed parent).



Suggested solution:

Create Thing instigatorThing = this.instigator ?? this.parent; and num before killing the parent such that these values are not null.

Code: [Select]
// RimWorld.CompExplosive
protected void Detonate(Map map)
{
if (this.detonated)
{
return;
}
this.detonated = true;
if (!this.parent.SpawnedOrAnyParentSpawned)
{
return;
}
if (map == null)
{
Log.Warning("Tried to detonate CompExplosive in a null map.");
return;
}
CompProperties_Explosive props = this.Props;
float num = props.explosiveRadius;
if (this.parent.stackCount > 1 && props.explosiveExpandPerStackcount > 0f)
{
num += Mathf.Sqrt((float)(this.parent.stackCount - 1) * props.explosiveExpandPerStackcount);
}
Thing instigatorThing = this.instigator ?? this.parent;
if (!this.parent.Destroyed)
{
this.parent.Kill(null);
}
if (props.explosionEffect != null)
{
Effecter effecter = props.explosionEffect.Spawn();
effecter.Trigger(new TargetInfo(this.parent.PositionHeld, map, false), new TargetInfo(this.parent.PositionHeld, map, false));
effecter.Cleanup();
}
ThingDef postExplosionSpawnThingDef = props.postExplosionSpawnThingDef;
float postExplosionSpawnChance = props.postExplosionSpawnChance;
int postExplosionSpawnThingCount = props.postExplosionSpawnThingCount;
GenExplosion.DoExplosion(this.parent.PositionHeld, map, num, props.explosiveDamageType, instigatorThing, null, null, null, postExplosionSpawnThingDef, postExplosionSpawnChance, postExplosionSpawnThingCount, props.applyDamageToExplosionCellsNeighbors, props.preExplosionSpawnThingDef, props.preExplosionSpawnChance, props.preExplosionSpawnThingCount);
}

7
This method exists:
Code: [Select]
// Verse.Thing
public virtual string LabelBaseCap
{
get
{
return this.LabelBase.CapitalizeFirst();
}
}

.. yet:
Code: [Select]
// RimWorld.Tradeable
public virtual string Label
{
get
{
return this.AnyThing.LabelBase.CapitalizeFirst();
}
}

.. while it should be called as this.AnyThing.LabelBaseCap:
Code: [Select]
return this.AnyThing.LabelBaseCap;
If a modded Thing wants to prepend something to any visible label it currently has to override LabelBase which means every other call (not
RimWorld.Tradeable) will be capitalizing the prepended text rather than the LabelBase text.

8
Code: [Select]
// RimWorld.StoryState.ctor
private int lastThreatBigQueueTick = -1;

(..)

// RimWorld.StoryState.ExposeData()
Scribe_Values.LookValue<int>(ref this.lastThreatBigQueueTick, "lastThreatBigQueueTick", 0, true);

Would probably make more sense as:

Code: [Select]
Scribe_Values.LookValue<int>(ref this.lastThreatBigQueueTick, "lastThreatBigQueueTick", -1, true);

9
Code: [Select]
// RimWorld.PawnRecentMemory.ctor
private int lastLightTick = 999999;
private int lastOutdoorTick = 999999;

(..)

// RimWorld.PawnRecentMemory.ExposeData()
Scribe_Values.LookValue<int>(ref this.lastLightTick, "lastLightTick", 0, false);
Scribe_Values.LookValue<int>(ref this.lastOutdoorTick, "lastOutdoorTick", 0, false);

Again, unspawned pawns cluttering savefiles with <last___Tick>999999</last____Tick>. Since spawned pawns will not have values of these
proportions it is only useful for pawns that have not been spawned yet.

Code: [Select]
Scribe_Values.LookValue<int>(ref this.lastLightTick, "lastLightTick", 999999, false);
Scribe_Values.LookValue<int>(ref this.lastOutdoorTick, "lastOutdoorTick", 999999, false);

10
Bugs / [A13] Scribe defaults: ThingContainer.maxStacks
« on: April 07, 2016, 05:22:36 PM »
Code: [Select]
// Verse.ThingContainer.ctor
private int maxStacks = 99999;

(..)

// Verse.ThingContainer.ExposeData()
Scribe_Values.LookValue<int>(ref this.maxStacks, "maxStacks", 0, false);

Another non logical scribe default, pawns use both an <inventory> (default maxStacks = 99999) and <carrier> (default maxStacks = 1), and a
default of 0 does not account for either and would result in a ThingContainer which does not accept anything since maxStacks is private.

New colony contained 5 of both <maxStacks>1</maxStacks> and <maxStacks>99999</maxStacks>.

11
Bugs / [A13] Savefile cluttering <stackCount>1</stackCount>
« on: April 07, 2016, 09:19:10 AM »
Code: [Select]
// Verse.Thing.ctor
public int stackCount = 1;

(..)

// Verse.Thing.ExposeData()
Scribe_Values.LookValue<int>(ref this.stackCount, "stackCount", 0, true);

Since this is set to 1 by default one could change that to:

Code: [Select]
Scribe_Values.LookValue<int>(ref this.stackCount, "stackCount", 1, true);
This was a redundancy of 83 occurences on a fresh colony due to pawns in other factions carrying single meals.

12
Bugs / [A13] Savefile cluttering <health>85</health>
« on: April 07, 2016, 09:09:19 AM »
Code: [Select]
Scribe_Defs.LookDef<ThingDef>(ref this.def, "def");

(..)

if (this.def.useHitPoints)
{
Scribe_Values.LookValue<int>(ref this.hitPointsInt, "health", -1, false);
}

Since def is already initialized at this point the default could be set to def.BaseMaxHitPoints (which might increase save times) or alternatively to 85 (replacing <health>85</health> in a fresh savefile yielded 35014 occurrences). A default of -1 would obviously never help loading/saving times.

All other <health> values (not 85 which has 35014 occurences) add up to 7053 occurrences:

  • 3026 times <health>120</health> (bushes)
  • 3489 times <health>300</health> (trees)
  • 538 other things (walls, equipment)

13
Bugs / [A13] Savefile cluttering <thickness>1</thickness>
« on: April 07, 2016, 08:18:14 AM »
Code: [Select]
<thing Class="Filth">
<def>RockRubble</def>
<id>RockRubble6583</id>
<pos>(88, 0, 1)</pos>
<thickness>1</thickness>
</thing>
<thing Class="Filth">
<def>RockRubble</def>
<id>RockRubble6585</id>
<pos>(87, 0, 0)</pos>
<thickness>1</thickness>
</thing>
<thing Class="Filth">
<def>RockRubble</def>
<id>RockRubble6586</id>
<pos>(89, 0, 1)</pos>
<thickness>1</thickness>
</thing>
<thing Class="Filth">
<def>RockRubble</def>
<id>RockRubble6587</id>
<pos>(88, 0, 0)</pos>
<thickness>1</thickness>
</thing>

etc etc etc

Since filth with a thickness of 0 are destroyed it seems counter intuitive to take 0 as the default value for the scribe, especially since a new colony on an average sized map contains 3539 instances of Filth with <thickness>1</thickness>.

Code: [Select]
Scribe_Values.LookValue<int>(ref this.thickness, "thickness", 0, false);
to

Code: [Select]
Scribe_Values.LookValue<int>(ref this.thickness, "thickness", 1, false);

14
Bugs / [A13] Savefile cluttering default(IntVec3) != IntVec3.Invalid
« on: April 07, 2016, 06:39:44 AM »
Every Thing's "pos" is saved since default(IntVec3) is somehow not equal to (-1000,-1000,-1000), since that is IntVec3.Invalid. Replace
the following code:

Code: [Select]
Scribe_Values.LookValue<IntVec3>(ref this.positionInt, "pos", default(IntVec3), false);

to

Scribe_Values.LookValue<IntVec3>(ref this.positionInt, "pos", IntVec3.Invalid, false);

Anything that is worn, stored or otherwise has no position value has this value saved, introducing one line of text for every one of those that
exist in the game environment (I managed to replace 65 occurences of <pos>(-1000,-1000,-1000)</pos> from a new colony's initial save).

This is still present in A13. For reference:

Code: [Select]
default(IntVec3) == new IntVec3(0,0,0);
IntVec3.Invalid == new IntVec3(-1000,-1000,-1000);

In A13 creating a new world and colony yielded me 185(!) occurences of <pos>(-1000,-1000,-1000)</pos>, this should be fixed as follows:

Code: [Select]
Scribe_Values.LookValue<IntVec3>(ref this.positionInt, "pos", IntVec3.Invalid, false);
.. which causes the default value to be assumed to be IntVec3.Invalid as its value is set in the constructor:

Code: [Select]
private IntVec3 positionInt = IntVec3.Invalid;


  • The same is the case for Faction.homeSquare (default(IntVec2) == (0,0); IntVec2.Invalid == (-1000,-1000))

15
Bugs / [A13] Double checking LanguageDatabase.activeLanguage == null
« on: April 07, 2016, 06:07:33 AM »
Verse.LanguageDatabase.LoadAllMetadata():

Code: [Select]
LanguageDatabase.defaultLanguage = LanguageDatabase.languages.FirstOrDefault((LoadedLanguage la) => la.folderName == LanguageDatabase.DefaultLangFolderName);
LanguageDatabase.activeLanguage = LanguageDatabase.languages.FirstOrDefault((LoadedLanguage la) => la.folderName == Prefs.LangFolderName);
if (LanguageDatabase.activeLanguage == null)
{
Prefs.LangFolderName = LanguageDatabase.DefaultLangFolderName;
LanguageDatabase.activeLanguage = LanguageDatabase.languages.FirstOrDefault((LoadedLanguage la) => la.folderName == Prefs.LangFolderName);
}
if (LanguageDatabase.activeLanguage == null || LanguageDatabase.activeLanguage == null)
{
Log.Error("No default language found!");
LanguageDatabase.defaultLanguage = LanguageDatabase.languages[0];
LanguageDatabase.activeLanguage = LanguageDatabase.languages[0];
}

Code: [Select]
if (LanguageDatabase.activeLanguage == null || LanguageDatabase.activeLanguage == null)
Better double check it, you never know with those public statics.

Still present in A13, I assume you'd want to check for defaultLanguage == null or remove the second activeLanguage check.

Pages: [1] 2