-
Notifications
You must be signed in to change notification settings - Fork 434
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes requested.
Apply to #870 as well.
Fixing after build events Fixing build parameters initialization Removing unnecessary nugets
Again, tests with relative directories ( |
Project now runs without any special problems. I'll now try to change to .NET core. |
The whole project still crash but build works
Runtime error:
|
Everything works except log4net logger. This logger seems to ignore every settings for printing, but yet recognize it. When I run it on debug mode I see some logging and then log seems to stop. |
Maybe hinders performance
@MythicManiac It should be safe to squash&merge now, be aware that we need to add EDIT: some tests are now broken cause path changed again |
The tests of ServerConsole get skipped as something in x86 causes problems. |
@MythicManiac I think it is safe to merge now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't see anything out of place really, if it works, it probably should be merged.
@@ -25,7 +24,7 @@ public class BaseTurret : ObjAiBase, IBaseTurret | |||
uint netId = 0 | |||
) : base(game, model, new Stats.Stats(), 50, x, y, 1200, netId) | |||
{ | |||
ParentNetId = Crc32Algorithm.Compute(Encoding.UTF8.GetBytes(name)) | 0xFF000000; | |||
ParentNetId = Force.Crc32.Crc32Algorithm.Compute(Encoding.UTF8.GetBytes(name)) | 0xFF000000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure why this was inlined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
VS marked the using
line as problematic for no reason so I just copied it here. I don't mind changing it back if VS do no problems with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah we'd rather have one more using directive than a line that's harder to read
[DeploymentItem("Content", "Content")] | ||
public void TestItemContentCollection() | ||
{ | ||
var collection = ItemContentCollection.LoadItemsFrom("Content/Data/LeagueSandbox-Default/Items"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file probably got deleted by mistake. I'll upload it back.
Added removed files that were removed by mistake
Travis can't run tests without stalling: microsoft/vstest#2080 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
What about instead of |
This PR continues #870 PR
Fixing travis build
Then upgrading to .NET Core
Resolve #869
Resolve #871
IMPORTANT:
To reserve credit to pipe01 after squash/merge I think you have to add Co-authored-by