feat(intro): Add short Intro Logo for The Super Hackers team#2267
feat(intro): Add short Intro Logo for The Super Hackers team#2267xezon wants to merge 7 commits into
Conversation
|
| Filename | Overview |
|---|---|
| Core/GameEngine/Include/GameClient/Intro.h | New header for Intro class; uses #pragma once, correct GPL + TheSuperHackers copyright, nullptr throughout, and enterNextState() correctly moved to private. |
| Core/GameEngine/Source/GameClient/Intro.cpp | New implementation of intro state machine; state loop correctly reaches IntroState_Done when no states are enabled, fade-in/out logic is guarded by m_fadeValue bounds, and doAsyncWait(3000) duration is correct. |
| GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp | Replaces ad-hoc intro/sizzle state booleans with the new Intro class; m_intro lifecycle (init/update/delete) is clean and the new draw() override correctly delegates to Intro::draw(). |
| Core/GameEngine/Source/GameClient/Display.cpp | Removes playLogoMovie() and all copyright-hold-time fields; stopMovie() and update() are simplified cleanly without any dangling references. |
| GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp | Replaces the dead 'end of video example code' comment with TheGameClient->DRAW() and removes the copyright display string rendering block. |
Reviews (12): Last reviewed commit: "fixup! feat(intro): Add short Intro Logo..." | Re-trigger Greptile
Additional Comments (1)
Fix by making Prompt To Fix With AIThis is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp
Line: 514:531
Comment:
**Intro can block startup**
`GameClient::update()` only shows the shell (`TheShell->showShellMap/showShell`) after `m_intro->isDone()`. With the new `Intro` implementation, if the global flags disable both intro and sizzle (`m_playIntro==false` and `m_playSizzle==false`) and `m_afterIntro` is not pre-set, `Intro` never reaches `Done` (see `IntroState_Start` handling), so the game can get stuck in the intro phase and never show the shell. This previously worked because `GameEngine::init()` forced `m_afterIntro = TRUE` when `m_playIntro` was false.
Fix by making `Intro` immediately become done when there are no enabled states, or by preserving the old `m_afterIntro` initialization behavior.
How can I resolve this? If you propose a fix, please make it concise. |
999721b to
0f278e6
Compare
0f278e6 to
1a738f7
Compare
|
Wouldn't it be more appropriate to show the Superhackers stuff in the credits menu? As is this would imply that the whole game is made by us. I don't think it would be fair to show our names more prominently than the original developers. |
Maybe remove all user names from this Intro and just show the Team name + url? We can also clarify "Edited by ..." |
I wouldn't add anything to the intro. I don't think it's necessary to remind the player every time the game starts who made the patch. Maybe just add a tiny note on the shellmap screen below the gamename "patched by Superhackers" or something like that. It's nice though that the 3-5 seconds black screen is removed here! |
|
I didn't even know what the 'black screen' was when I saw this PR, because I have renamed the files for the EA logo and sizzle and I don't see it. This makes the game launch faster, and saves me 5+ seconds every time I launch the game. I strongly dislike changing anything that would unnecessarily increase the launch time for me as developer. I'm ok with it if I can skip it with some command line. That said, I like the new intro. |
|
I don't think people will understand what it means if the intro screen is this plain. On the intro, maybe add 3 to 5 sentences with the key achievements of TheSuperHackers. Maybe something like: |
I feel like adding too much text won't really be very useful given the fact that it's only shown for 3 seconds. |
I agree with this sentiment. What we can add is a "Improved by" on top, to clarify that this Author has improved the game. This is very simple and broad. |
|
When this is merged I will make a follow up to skip the entire intro sequence with a single ESC press. This will alleviate any waiting pains for impatient users. |
1a738f7 to
29b1ce7
Compare
Additional Comments (1)
Prompt To Fix With AIThis is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp
Line: 119:119
Comment:
Missing cleanup of `m_intro` in destructor - potential memory leak if `GameClient` is destroyed before intro completes.
```suggestion
delete m_intro;
m_intro = nullptr;
```
How can I resolve this? If you propose a fix, please make it concise. |
29b1ce7 to
9c82551
Compare
|
Maybe text can be made a bit bigger. |
|
A suggestion: the credits screen would display all contributors names. Something like: The |
|
How about some old school ASCII text? Like the Atari days. |
832f3d8 to
f3a985c
Compare
Maybe is a bit difficult to read. Also makes it look like hackware. Rebased. |
|
I've checked this locally and it rebases cleanly onto the current main branch with no conflicts. LGTM for merging (I think the pretext of Additionally, I wonder if instead of hardcoding this into the game engine, wouldn't it be better to create an external file like the original game intro |
|
We cannot put a BIK video because it would not work in forks of this project and requires a data patch. The intro is intentionally hard coded, because it is not meant to be changed, other than the localization. |
f3a985c to
bf25fd5
Compare
|
Maybe something like "open source community" should be added to attract developers |
|
Needs rebase and review. |
|
|
||
| if (TheGlobalData->m_playSizzle) | ||
| { | ||
| if (TheGlobalData->m_playIntro) |
There was a problem hiding this comment.
nit: style, always use braces, several times in this file: line 55, 67, 102, 219, 274
There was a problem hiding this comment.
For small conditions it is generally ok when it reads ok.
|
Oh wow why does the ASCII text fall apart like this? |
| e.displayString->getSize(&pos.x, &pos.y); | ||
| pos.x = (screenWidth / 2) - (pos.x / 2); | ||
| pos.y = (Int)(e.screenHeightFactor * screenHeight); | ||
| e.displayString->setWordWrap(screenWidth); |
There was a problem hiding this comment.
nit: could we set this once? or do we want to do it every frame?
There was a problem hiding this comment.
The initial intent was to make it auto adjust on resolution change during display but that would not work anyway so I moved it now.
| { | ||
| const DisplaySetting& s = settings[i]; | ||
| DisplayEntity& e = m_displayEntities[i]; | ||
| e.sizeX = Int(s.sizeX * fontScale); |
There was a problem hiding this comment.
nit: is this more general scaling than just font? Maybe uiScale?
| { | ||
| DisplaySetting() | ||
| : imageName(nullptr) | ||
| , font("Arial") |
There was a problem hiding this comment.
We set this default but then use Lucida Console everywhere. Maybe just set it here?
There was a problem hiding this comment.
The rationale for this was that the common default font in the game for latin languages is Arial.
|
I left some nits, and it needs another rebase, but it looks good to me. I think ESC should work to skip, and/or a -quickstart arg would be nice. |
Oof that's no good. I'm in favor of using an image instead of ascii. If we need to troubleshoot the ascii, claude says "pick a much larger sizeY (so GDI's full-block glyph actually fills its cell), guarantee the font is installed (or use one shipped with the game), and stop the setWordWrap call from running every draw" - but I still think an image is easier and safer |
…movie code (#2267)
bf25fd5 to
2679b54
Compare
I changed the font. Please try again. A mono font should work. That said, the previous font was supposed to be mono, but maybe it is not on all systems.
www. removed.
There is I tried triggering it but that did not work, probably because it is a positional sound. Would require more effort to get working.
I expect that many players could perceive this as "gay wording", because "lovingly" does not fit the theme of the game. |







Merge with Rebase
This change has 5 commits:
Old Intro Sequence
(*1) I did not measure how long the black screen was. According to code somewhere between 3 to 5 seconds.
New Intro Sequence
TODO