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

Bring Luomu's game msg log over from the SSP fork. #2527

Merged
merged 13 commits into from Aug 3, 2014

Conversation

fluffyfreak
Copy link
Contributor

Description:

@Luomu wrote a much nicer msg logging service that doesn't interrupt the scanner, can display multiple messages and does a nice slow fade out. Since he's busy with real life and a house without stairs he's given me permission to bring it over.

Notes:

You might notice some stuff in the Colors.h etc files mentioning IFF, don't worry about this for now it was just easier to leave it in with this commit than pick it out only to add it in subsequent commits that I'm in the process of preparing.

Big thanks should go to @Luomu for this, not me, I'm just copying n' pasting ;)

@robn
Copy link
Member

robn commented Nov 5, 2013

I'm not against this is principle (haven't looked at the code or tested yet), but I am wary of doing anything with the old UI since its all going to be replaced. Though I know its taking a while and this doesn't touch much, so it might be ok. What do you think?

Can you split this out into separate logical commits? Or even pick @Luomu's original commits?

@fluffyfreak
Copy link
Contributor Author

The IFF bits are literally just their definitions in colors.h/cpp the rest is just what's required for the new msg stuff and it actually breaks it out of the control panel using the new UI so it's forward not backward looking :)

@robn
Copy link
Member

robn commented Nov 5, 2013

Apart from getting the font from the UI context, I don't see any use of the new UI. I do see direct calls into the text renderer.

I also see calls into the renderer to change depth attributes etc. I'm not saying they're unnecessary, but because there's no related commit message I can't tell what they're actually for.

And there's also build changes, they don't belong in the same commit.

@robn
Copy link
Member

robn commented Nov 5, 2013

I guess I've got half an hour right now, I'll see what I can do with this over breakfast.

@fluffyfreak
Copy link
Contributor Author

Build changes? Ah of course, I didn't do the makefiles!
The depth write and test calls are just necessary evils because the state doesn't get set in the right places and this exposes that we've been getting away with it for a while. Switching to render targets and the other stuff all cause the same issues sadly. Yeah I should have put them in separately.

Want me to fixup the makefiles before bed whilst you enjoy breakfast :)

@robn
Copy link
Member

robn commented Nov 5, 2013

Build files are MSVC project file updates too. I don't expect you to do the Makefiles, that's my problem. But having MSVC project changes in the same commit as the code sucks.

@fluffyfreak
Copy link
Contributor Author

Really, one won't work without the other though? Well, I suppose I could have added the files, modified the project, then done the hookup in 3 separate commits.
I could do something like that for the other stuff I'll bring across if that'd help?

@robn
Copy link
Member

robn commented Nov 5, 2013

Don't worry about it. I think I'm overreacting a bit because its everything in one commit and can't see what relates to what else.

Back to the original question. Is it worth doing this now? I'm inclined to think its harmless enough, though I expect most of it will be ripped out when WorldView is moved to the new UI.

@fluffyfreak
Copy link
Contributor Author

Worth it? Hmm, well I was reminded of it and the other changes the other day when I found myself wanting to bite through my keyboard in frustration at the dribble of messages dropping me out of time-accel every few seconds.

Yeah, it's a nice change, it looks much nicer, and it'll be cleaner to rewrite when the time comes - and until the time comes, it'll also suck less than the current system :)

@fluffyfreak
Copy link
Contributor Author

Oh and i'll try to make the other commits cleaner / more-staged anyway :)

@robn
Copy link
Member

robn commented Nov 5, 2013

I tend to agree with that. Ok :)

(must do more work on the station screens on the train ride in today)

@robn
Copy link
Member

robn commented Nov 5, 2013

Linux build fixes on robn/msglog.

I'm going to try some different positions and font sizes, but that's not a big deal.

All but the bottom line are being clipped when the station screen is showing. It also probably isn't going to play nice when the info screens are up. My plan once everything is on new-ui was to have something like this in the worldview only, and reporting from the other screens (buy/sell failures etc) done in UI dialogs or other elements. I'm not saying we should do that now, but it'd be nice if new additions didn't make things worse in the interim.

I'm not sure what to do here. I really want to like this one!

@fluffyfreak fluffyfreak mentioned this pull request Nov 6, 2013
@fluffyfreak
Copy link
Contributor Author

I could maybe move the render order so that it's done after the GUI, then it'd draw over the top on the station and other screens.
Or you could move it down that it's drawn over the cpan? You'd still be able to see the cpan mostly with the messages scrolling over the top of it.

Conflicts:
	src/StationShipViewForm.cpp
	win32/vc2010/pioneer.vcxproj.filters
	win32/vc2012/pioneer.vcxproj.filters
	win32/vc2013Express/pioneer.vcxproj.filters
@fluffyfreak
Copy link
Contributor Author

re-merged, resolved conflicts etc.

…nto LuomuMsgLog

Conflicts:
	src/LuaChatForm.cpp
	src/Makefile.am
	src/MarketAgent.cpp
	src/Player.cpp
	src/SpaceStation.cpp
	src/StationCommodityMarketForm.cpp
	src/StationPoliceForm.cpp
	src/StationShipEquipmentForm.cpp
	src/StationShipRepairForm.cpp
	src/StationShipViewForm.cpp
	win32/vc2012/pioneer.vcxproj
	win32/vc2013Express/pioneer.vcxproj
@fluffyfreak
Copy link
Contributor Author

Took a little bit of kicking to get this working again since there's 2 months of changes. It's not the best system, but it's better than our existing one so I still think that it's worth having until such time as we get a new one which works the way it should.

@walterar
Copy link
Contributor

walterar commented Jan 1, 2014

I'm testing this.

This is one of the essential things that should be first on the list. If we want the work of game-tester less frustrating. Task Less frustrating = more game testers

Only two problems, until now. The font size is very small. I tried FONT_SMALLEST = FONT_NORMAL (in Widget.h), and work well. I guess there are better way to do this.

The other problem is that the text of Tombstone disappeared.

@fluffyfreak
Copy link
Contributor Author

Thanks @walterar, I'm sure @Luomu would be glad to hear that, it's been driving me crazy for ages as well :)

@robn
Copy link
Member

robn commented Feb 4, 2014

I've got my own version of this coming down the pipes right now. It's a perfect feature for getting the new UI into the worldview and experimenting with layouts and such. It should be good to go in a couple of days.

@fluffyfreak
Copy link
Contributor Author

ok cool, I'll close this then :)

@fluffyfreak fluffyfreak closed this Feb 4, 2014
…nto LuomuMsgLog

Conflicts:
	src/Makefile.am
	src/Pi.cpp
	src/Pi.h
	src/SectorView.cpp
	src/Ship.cpp
	src/ShipCpanel.cpp
	src/ShipCpanelMultiFuncDisplays.cpp
	src/WorldView.cpp
	win32/vc2010/pioneer.vcxproj
	win32/vc2010/pioneer.vcxproj.filters
	win32/vc2012/pioneer.vcxproj
	win32/vc2012/pioneer.vcxproj.filters
	win32/vc2013Express/pioneer.vcxproj
	win32/vc2013Express/pioneer.vcxproj.filters
State ticket, setting a render state and a couple of places that added new messages.
@fluffyfreak
Copy link
Contributor Author

This might be controversial but since nothing else ever came along to replace this I've updated it to latest master (code will need reviewing) and am reopening it for discussion.

@fluffyfreak fluffyfreak reopened this Jul 30, 2014
@fluffyfreak
Copy link
Contributor Author

@robn PING! ;)

@impaktor
Copy link
Member

src/GameLog.cpp:75: undefined reference to `Colors::HUD_MESSAGE

@fluffyfreak
Copy link
Contributor Author

Is that Linux or Windows? I built it last night using vs2013 but I had to guess at the Linux makefile as I don't have a machine to test it on at the moment.

@fluffyfreak
Copy link
Contributor Author

Ok it was missing a cpp file, I think I've added it back in correctly but cannot test.

@impaktor
Copy link
Member

impaktor commented Aug 2, 2014

👍

People: protest now, or forever hold your peace.

@robn
Copy link
Member

robn commented Aug 2, 2014

Abstaining from the vote. I think it's wrong to put effort into maintaining an old-UI system, and time would be better spent finishing the new comms panel. But if I'm not willing or able to do something right now then there's certainly no way I'd insist someone else do it.

@fluffyfreak
Copy link
Contributor Author

👍
We might get a new-UI based on eventually, and that effort certainly needs to be made one day.
This is here, right now, and works. So why not have it until something better comes along?

@robn
Copy link
Member

robn commented Aug 3, 2014

Not hearing any protests :)

@Zireael07
Copy link

How quick the new UI would come? If very soon, then by all means, abandon it.
If the new UI is light years away, please merge it.

@fluffyfreak
Copy link
Contributor Author

Mergey mergy merged

fluffyfreak added a commit that referenced this pull request Aug 3, 2014
Bring Luomu's game msg log over from the SSP fork.
@fluffyfreak fluffyfreak merged commit dc29865 into pioneerspacesim:master Aug 3, 2014
@fluffyfreak fluffyfreak deleted the LuomuMsgLog branch August 3, 2014 19:36
@impaktor
Copy link
Member

impaktor commented Aug 4, 2014

<dv-> now there's a Color.cpp and a Colors.cpp

@radius75
Copy link
Contributor

cosmetic:
screenshot-20140813-182744
screenshot-20140813-183230

@impaktor
Copy link
Member

So only the two strings beloning to StationRefuelling.lua has the extra ":". Strange.

@radius75
Copy link
Contributor

no "extra", lack station name (Moscow: Welcome aboard Moscow. Your...)

Add(stringf("%0: %1", from, msg));

@fluffyfreak
Copy link
Contributor Author

Open a new issue don't comment on closed pull requests because they might not get seen.

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

6 participants