Skip to content

Commit

Permalink
Trying to make workaround for xrGame CTD.
Browse files Browse the repository at this point in the history 
Also enabled autosave.
  • Loading branch information
@andrew-boyarshin
andrew-boyarshin committed Oct 5, 2014
1 parent f517280 commit 7309acb
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 11 deletions.
44 changes: 35 additions & 9 deletions src/xrGame/Entity.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,18 +322,44 @@ DLL_Pure *CEntity::_construct ()

const u32 FORGET_KILLER_TIME = 180000;

void CEntity::shedule_Update (u32 dt)
void CEntity::shedule_Update(u32 dt)
{
inherited::shedule_Update (dt);
if (!getDestroy() && !g_Alive() && (m_killer_id != u16(-1))) {
if (Device.dwTimeGlobal > m_level_death_time + FORGET_KILLER_TIME) {
m_killer_id = u16(-1);
NET_Packet P;
u_EventGen (P,GE_ASSIGN_KILLER,ID());
P.w_u16 (u16(-1));
if (IsGameTypeSingle()) u_EventSend (P);
inherited::shedule_Update(dt);
BOOL lDestroy = TRUE;
BOOL lAlive = TRUE;
BOOL lKillerId = TRUE;
try {
lDestroy = getDestroy();
}
catch (std::exception& e) {
Msg("RELEASE CONFIGURATION FIX: lDestroy failed");
}
try {
lAlive = g_Alive();
}
catch (std::exception& e) {
Msg("RELEASE CONFIGURATION FIX: lAlive failed");
}
try {
lKillerId = m_killer_id != u16(-1);
}
catch (std::exception& e) {
Msg("RELEASE CONFIGURATION FIX: lKillerId failed");
}
try {
if (!lDestroy && !lAlive && lKillerId) {
if (Device.dwTimeGlobal > m_level_death_time + FORGET_KILLER_TIME) {
m_killer_id = u16(-1);
NET_Packet P;
u_EventGen(P, GE_ASSIGN_KILLER, ID());
P.w_u16(u16(-1));
if (IsGameTypeSingle()) u_EventSend(P);
}
}
}
catch (std::exception& e) {
Msg("RELEASE CONFIGURATION FIX: if block failed");
}
}

void CEntity::on_before_change_team ()
Expand Down
4 changes: 2 additions & 2 deletions src/xrGame/autosave_manager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ float CAutosaveManager::shedule_Scale ()
void CAutosaveManager::shedule_Update (u32 dt)
{
inherited::shedule_Update (dt);
#pragma todo("Plecha to Plecha : AUTOSAVE (do not forgive to enable it in release version:-))))!!!!")
if (true) return;
//#pragma todo("Plecha to Plecha : AUTOSAVE (do not forgive to enable it in release version:-))))!!!!")
//if (true) return;
if (!ai().get_alife())
return;

Expand Down

10 comments on commit 7309acb

@nitrocaster
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert this commit along with the next one. Here's why:

  • conventions
  • lots of redundant code

What's the problem with autosave you think you've fixed? It just works in vanilla release. Even if there's a problem, you should fix it in the separate commit.

@nitrocaster
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another thing: have you found exact reason of the crash? What is the fix here?

@nitrocaster
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also: since @awdavies have assigned #11 to himself, you should ask him first if he don't mind if you take his task.

@andrew-boyarshin
Copy link
Contributor Author

@andrew-boyarshin andrew-boyarshin commented on 7309acb Oct 5, 2014 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrew-boyarshin
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nitrocaster Please, explain, which conventions I broke. For example, variable naming - should conform.
I also don't think that's a good fix.
And autosave: as you can see, autosave every N minutes of gaming is disabled. I've enabled it.

@andrew-boyarshin
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nitrocaster Okay, opening brackets. Will fix that.

@nitrocaster
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After making try and catch blocks, it works.

How do you know that? It worked before and crashed occasionally. The code you wrapped doesn't throw exceptions you're trying to catch.

I dunno what to do. But it works.

In that case, work on your own tasks, awdavies assigned himself on this. If you don't know the reason of crash - you can't really fix it.

For conventions: tabs and indentation. Open and close brackets of a section should be on the same level (though there's no exact rule for try..catch.. blocks, it's still a 'section').

And autosave: as you can see, autosave every N minutes of gaming is disabled. I've enabled it.
Have you tested it? Maybe every N minutes autosave was considered redundant since there are quicksaves + 'smart autosaves'.

@nitrocaster
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also once thing I haven't yet mentioned in conventions. You should prefer cross-platform types over platform-specific ones

BOOL lDestroy = TRUE;

Since we use C++ with its nice bool type, we can simply write:

bool destroy = true;

@andrew-boyarshin
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we use C++ with its nice bool type, we can simply write:

I don't think so. BOOL is declared as int there. And TRUE is 1.

Maybe every N minutes autosave was considered redundant since there are quicksaves + 'smart autosaves'.

In my opinion, it is not considered redundant. TODO says, that it should me enabled in Release version.

How do you know that? It worked before and crashed occasionally. The code you wrapped doesn't throw exceptions you're trying to catch.

CTD was permanent on Release configuration, so it always crashed, and not occasionally. So, if I see, that I can play with that workaround for about 5 minutes(then, I simply left the game), then, I think, I can be sure, that it works.

In that case, work on your own tasks, awdavies assigned himself on this. If you don't know the reason of crash - you can't really fix it.

I'm really sorry for interruption of work, but I've fixed it, because I couldn't test my changes, and it works. Maybe, @awdavies will find out better way to fix that issue(and revert that commit), but for now it works.

@nitrocaster
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I said, you should not use windows-specific types when you can use cross-platform ones. BOOL is windows-specific (in addition, it breaks conventions).

In my opinion, it is not considered redundant.

Well, at least one should be able to disable it. Better - discuss it with other collaborators.

I asked you if you have found the reason of crash. Have you?

because I couldn't test my changes, and it works.

This is what are branches for.

Let it be as it is, but next time consider following:

  • discuss changes that may be disputable (alternatively, make such changes in separate branch)
  • double check if your code is compliant with conventions we use
  • don't make '2 in 1' commits (in this particular case you have bugbix and new feature)

Please sign in to comment.