Сделать загрузку капчи асинхронной #115
Conversation
BoomEaro
commented
Jun 13, 2022
•
BoomEaro
commented
- Капча загружается параллельно с загрузкой банжикорда и с пониженным приоритетом.
- Каждую секунду при генерации капчи, генератор обновляет список капчи, что позволит игрокам заходит на сервер во время генерации.
- Добавлена команда /botfilter generate для принудительной ручной регенерации капчи.
- Капча сама регенерируется каждые 6 часов. (возможно редко, это стоит еще изучить)
- Сообщение о генерации капчи показываются только когда банжикорд был полностью загружен, что исключает путаницу логов.
- На каждый поток используется свой рандомный генератор (ThreadLocalRandom)
| @@ -113,6 +113,9 @@ public class BungeeCord extends ProxyServer | |||
| * Current operation state. | |||
| */ | |||
| public volatile boolean isRunning; | |||
| //BotFilter | |||
| @Getter | |||
| private volatile boolean enabled; | |||
There was a problem hiding this comment.
Why volatile? I don't see the use of it in this scenario
| { | ||
| CaptchaGeneration.generateImages(); | ||
| } catch ( CaptchaGenerationException ignored ) |
There was a problem hiding this comment.
Don't just ignore Exceptions. Either provide a config option for that or print a message every time
There was a problem hiding this comment.
Don't just ignore Exceptions. Either provide a config option for that or print a message every time
The main reason why the exception is ignored here is that when the user executes the /botfilter reload command, it may happen that at that moment the captcha is already in the process of being generated (for example, execute the /botfilter reload command twice), in this case it makes no sense to show the stacktrace to the user in which has no information at all.
I think this problem can be solved if you forcibly stop the generation and start it again
| @@ -52,6 +55,16 @@ public void execute(CommandSender sender, String[] args) | |||
| { | |||
| export( sender, args ); | |||
| sender.sendMessage( "§aКоманда выполнена" ); | |||
| } else if ( args[0].equalsIgnoreCase( "generate" ) ) | |||
There was a problem hiding this comment.
For what is the command? The captchas are being generated from the startup anyway
There was a problem hiding this comment.
For what is the command? The captchas are being generated from the startup anyway
Custom ability to regenerate captcha at any time. There is an option for automatic regeneration, so I thought it would be nice to manually regenerate when needed.
There was a problem hiding this comment.
I don't know if you should really add this. Some people might don't get it and just spam the command because nothing happens. You could at least provide some type of boolean so that you can't spam it until it is done.
There was a problem hiding this comment.
I don't know if you should really add this. Some people might don't get it and just spam the command because nothing happens. You could at least provide some type of boolean so that you can't spam it until it is done.
There is an exception interception, and an error message output to the user if the captcha is already in the process of being generated. The command is only for the console, players cannot execute it
| private static final int PACKETID_18 = 0x34; | ||
| private static final int PACKETID_19and119 = 0x24; | ||
|
|
||
| private static final int PACKETID_113and114and116 = 0x26; | ||
| private static final int PACKETID_115and117 = 0x27; | ||
| private static final int PACKETID_1162 = 0x25; | ||
| private final Random random = new Random(); |
| { | ||
| Thread.sleep( 1000L ); | ||
| } catch ( InterruptedException ex1 ) | ||
| List<Font> fonts = Arrays.asList( |
There was a problem hiding this comment.
It'd be great to make the Fonts configurable in the config
There was a problem hiding this comment.
It'd be great to make the Fonts configurable in the config
This is a good idea, however, this pull request is specifically about asynchronous captcha generation, adding other features, as I understand it, should be in other pull requests, since the idea with asynchronous captcha may not be liked by the author of the project.
There was a problem hiding this comment.
Yea, it was just a recommendation :P
|
|
||
| //Окончательно устанавливаем оставшиеся капчи | ||
| PacketUtils.captchas.setCaptchas( new ArrayList<>( holders ) ); | ||
| System.gc(); |
There was a problem hiding this comment.
I wouldn't all the GC tbh
The captcha generation behavior is identical to how it was, including the call to System.gc(), and there is nothing bad about it, in fact, a lot of memory is consumed during generation, it's not a problem to always call the garbage collector at the end
There was a problem hiding this comment.
I don't like the idea in general: https://stackoverflow.com/questions/5086800/java-garbage-collection
|
And could you provide some testing results? Like before and after (time in ms)? This would be great. |
Captcha generation performance has not changed, the only advantage is that you do not have to wait for captcha generation at the very beginning before players can join to BungeeCord. |
| public class CaptchaPainter | ||
| { | ||
|
|
||
| private final int width = 128; | ||
| private final int height = 128; | ||
| private final Color background = Color.WHITE; | ||
| private final Random rnd = new Random(); | ||
| private final Random rnd; |
There was a problem hiding this comment.
This will just throw NPE's, please fix it!
There was a problem hiding this comment.
This will just throw NPE's, please fix it!
The class is annotated lombok with @requiredargsconstructor annotation
It generates a constructor where there is Random
New CaptchaPainter instances are created in the CaptchaGenerationTask class always with Random instances inside
