Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added compatibility for Linux platform #5

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Scienziatogm
Copy link
Contributor

Added compatibility for Linux Platform. Tested on x86_64 ubuntu dedicated server.

Copy link
Member

@neico neico left a comment

Choose a reason for hiding this comment

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

I'd really would like to avoid adding a third-party lib just for managing console output, but I'm open to get educated as I didn't find the time to look into the matter on linux yet (I only know about the ANSI control codes which should work on both linux and mac)

Warning: I only skimmed over it quickly, so I might have missed quite a few spots, hopefully once you address my comments there'll be a more thorough review.

Source/DedicatedServer/DedicatedServer.Build.cs Outdated Show resolved Hide resolved
Source/DedicatedServer/Private/ServerConsole.cpp Outdated Show resolved Hide resolved
Source/DedicatedServer/Private/ServerConsole.cpp Outdated Show resolved Hide resolved
Source/DedicatedServer/Private/ServerConsole.cpp Outdated Show resolved Hide resolved
Source/DedicatedServer/Private/ServerConsole.cpp Outdated Show resolved Hide resolved
Source/DedicatedServer/Private/ServerConsoleLinux.cpp Outdated Show resolved Hide resolved
Source/DedicatedServer/Private/ServerConsoleLinux.cpp Outdated Show resolved Hide resolved
Source/DedicatedServer/Private/ServerConsoleLinux.cpp Outdated Show resolved Hide resolved
Source/DedicatedServer/Private/ServerConsoleLinux.cpp Outdated Show resolved Hide resolved
Source/DedicatedServer/Private/ServerConsoleLinux.cpp Outdated Show resolved Hide resolved
@neico
Copy link
Member

neico commented Mar 11, 2020

@Scienziatogm needs a rebase, and there's still 2 unresolved conversations

@@ -284,7 +284,7 @@ void DumpConsoleHelp();

COORD FServerConsole::GetCursorPosition()
{
COORD hCursorPosition = 0;
COORD hCursorPosition = { 0, 0 };
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
COORD hCursorPosition = { 0, 0 };
COORD hCursorPosition = {};

}
if ( hInputEvents == '\n' )
{
auto& sInput = m_sInput;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
auto& sInput = m_sInput;
auto sInput = m_sInput;

Not sure if we need a reference here

Comment on lines +235 to +237
COORD retval;
getyx( stdscr, retval.Y, retval.X );
return retval;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
COORD retval;
getyx( stdscr, retval.Y, retval.X );
return retval;
COORD hPos;
getyx( stdscr, hPos.Y, hPos.X );
return hPos;

Comment on lines +242 to +244
int retval = move( hCursorPosition.Y, hCursorPosition.X );
refresh();
return retval == OK ? true : false;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
int retval = move( hCursorPosition.Y, hCursorPosition.X );
refresh();
return retval == OK ? true : false;
int iStatus = move( hCursorPosition.Y, hCursorPosition.X );
refresh();
return iStatus == OK;

Comment on lines +30 to +34
COORD()
{
this->Y = 0;
this->X = 0;
}
Copy link
Member

Choose a reason for hiding this comment

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

Is this even needed?

Comment on lines +24 to +28
COORD( int Y, int X )
{
this->Y = Y;
this->X = X;
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
COORD( int Y, int X )
{
this->Y = Y;
this->X = X;
}
COORD( int iX, int iY )
{
X = iX;
Y = iY;
}

Make sure you validate the order of the parameters on the rest of the code (with or without accepting this suggestion)

@neico neico mentioned this pull request Mar 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants